tarantool-php / client

PHP client for Tarantool.
MIT License
67 stars 22 forks source link

Tarantool 3.0 SQL breaking change compatibility #84

Closed DifferentialOrange closed 1 year ago

DifferentialOrange commented 1 year ago

Add compatibility layer to SQL scan queries

Scanning SQL queries will be forbidden in Tarantool 3.0 by default [1]. To use them, SEQSCAN keyword must be specified before space name. One may also set compat.sql_seq_scan_default to "old", but implicit scanning will be forbidden in future versions nonetheless.

Breaking change will be introduced in [2]. We need to merge this PR to PHP connector so [2] could pass integration tests before merge.

  1. https://github.com/tarantool/tarantool/commit/77648827326ad268ec0ffbcd620c2371b65ef2b4
  2. https://github.com/tarantool/tarantool/pull/8602

Make Tarantool 2.11 the default image in CI

Tarantool 2.11 is an LTS release of Tarantool 2.x series.

Totktonada commented 1 year ago

I was about to send the following patch:

diff --git a/tests/Integration/client.lua b/tests/Integration/client.lua
index ebaa321..e94d5fe 100755
--- a/tests/Integration/client.lua
+++ b/tests/Integration/client.lua
@@ -1,5 +1,26 @@
 #!/usr/bin/env tarantool

+-- Allow sequencial scanning in SQL queries.
+--
+-- The test suite is run on different Tarantool versions.
+--
+-- The old versions have no SEQSCAN keyword, so we can't use it
+-- unconditionally. A conditional usage is possible, but it
+-- requires an extra logic to form an SQL request.
+--
+-- Let's just enable the sequencial scanning globally for testing
+-- purposes.
+--
+-- Beware: applications usually shouldn't do it.
+--
+-- https://github.com/tarantool/tarantool/pull/8064
+-- https://github.com/tarantool/tarantool/pull/8283
+-- https://github.com/tarantool/tarantool/pull/8602
+local ok, compat = pcall(require, 'compat')
+if ok then
+    compat.sql_seq_scan_default = 'old'
+end
+
 local listen = os.getenv('TNT_LISTEN_URI')

 box.cfg {

However, your patchset solves the same problem in a better way.

DifferentialOrange commented 1 year ago

I was about to send the following patch:

diff --git a/tests/Integration/client.lua b/tests/Integration/client.lua
index ebaa321..e94d5fe 100755
--- a/tests/Integration/client.lua
+++ b/tests/Integration/client.lua
@@ -1,5 +1,26 @@
 #!/usr/bin/env tarantool

+-- Allow sequencial scanning in SQL queries.
+--
+-- The test suite is run on different Tarantool versions.
+--
+-- The old versions have no SEQSCAN keyword, so we can't use it
+-- unconditionally. A conditional usage is possible, but it
+-- requires an extra logic to form an SQL request.
+--
+-- Let's just enable the sequencial scanning globally for testing
+-- purposes.
+--
+-- Beware: applications usually shouldn't do it.
+--
+-- https://github.com/tarantool/tarantool/pull/8064
+-- https://github.com/tarantool/tarantool/pull/8283
+-- https://github.com/tarantool/tarantool/pull/8602
+local ok, compat = pcall(require, 'compat')
+if ok then
+    compat.sql_seq_scan_default = 'old'
+end
+
 local listen = os.getenv('TNT_LISTEN_URI')

 box.cfg {

However, your patchset solves the same problem in a better way.

As far as I understand, in some future release (like 4.x) this compat option will be removed. Is that how it works?

Totktonada commented 1 year ago

As far as I understand, in some future release (like 4.x) this compat option will be removed. Is that how it works?

I suppose that it works this way.

rybakit commented 1 year ago

@DifferentialOrange @Totktonada Thank you for your contribution!