Fixes gh-2206 Behavior changes in SQL #2350

pgulutzan · 2021-09-26T22:38:18Z

@ImeevMA: I am suggesting you as a reviewer, but if you think it is more appropriate for other people it can be changed to whatever you think is reasonable. Since it is a large set of changes, it might be proper to have more than one dev reviewer. Since it is for 2.10 and the "release" page is not officially done yet, there is no rush. (That is why I have not yet added @NickVolynkin as a reviewer, but it will happen someday.) Of course this is related to issue #2296.

Some new comments:

We will say "numeric" rather than "number" in some places. It's clumsy, but otherwise people will associate "number" with "NUMBER".
The alternative is to say, repeatedly, "number (but not NUMBER)".
I made a mistake saying "always round down". I see now that rounding is toward zero.
In reference_sql/sql_features.rst: I decided to make no change. Although version 2.10 is a backward step with less standard compatibility than before, the examples for E011-04 and E011-06 still work. I noticed by chance that ALTER TABLE ... ADD COLUMN has been working for a while, so I changed that.

Some more new comments: regarding fourth commit:
Default function parameter types are at the end of SQL Statements and Clauses.
There are replies for all comments, although a few haven't been marked yet as resolved.

Resolves #2295, #2296, #2298

ImeevMA

Hi! I believe all of the problems mentioned in #2296 have been documented here. See 11 comments below. I also want to say that some changes will be changed again in the next release.

doc/reference/reference_sql/sql_statements_and_clauses.rst

doc/reference/reference_sql/sql_user_guide.rst

ImeevMA · 2021-10-01T15:57:49Z

doc/reference/reference_sql/sql_user_guide.rst

 non-integer results.
 Example: ``5 / 2``, result = 2.

 ``%`` modulus (arithmetic)
-Divide second number into first number according to standard arithmetic rules.
+Divide second numeric into first numeric according to standard arithmetic rules.
 The result is the remainder.
 Example: ``17 % 5``, result = 2.

 ``<<`` shift left (arithmetic)


All operands must be positive INTEGER or UNSIGNED. And here the implicit cast is helpless. The same for all bit-wise operations.

Okay for %. And for the bit-wise operations, I'm changing "unsigned" to "non-negative INTEGER or UNSIGNED".

For % INTEGER could be of any sign, for example:

tarantool> box.execute([[SELECT -123 % 4;]]) --- - metadata: - name: COLUMN_1 type: integer rows: - [-3] ... tarantool> box.execute([[SELECT 123 % -4;]]) --- - metadata: - name: COLUMN_1 type: integer rows: - [3] ...

Okay, I added Example: -123 % 4, result = -3.

I still see

The result is the remainder. Starting with Tarantool version 2.10.1, operands must be positive INTEGER or UNSIGNED.

doc/reference/reference_sql/sql_user_guide.rst

NickVolynkin · 2021-10-11T08:47:18Z

@pgulutzan is this PR complete and ready for translation?

pgulutzan · 2021-10-11T18:02:45Z

@NickVolynkin: This PR is not ready for translation. I think that my replies to @ImeevMA are probably adequate but will not consider the issues "resolved" until he says okay. I see that @igormunkin is also a reviewer now, and I will add you.

ImeevMA

Thanks for fixes I added 4 more comments, but I think 3 of them are not essential. The only comment that matters is the one about % operands.

ImeevMA · 2021-10-20T16:35:22Z

doc/reference/reference_sql/sql_statements_and_clauses.rst

-the result will be 3,
-because X'41' is the third byte in the byte sequence.
+
+Starting with Tarantool version 2.10, arguments with data type VARBINARY are illegal.


Actually, now I'm working on bringing back to the POSITION() an ability to work with VARBINARY. Though I'm not sure if it's worth mentioning this in the doc right now.

You mean: it is only illegal in 2.10 beta, but in earlier and later versions it is legal?
In that case, I think, it is like a temporary bug: it's better to not mention it at all,
So I suggest: I will go back to the text in
https://www.tarantool.io/en/doc/latest/reference/reference_sql/sql_statements_and_clauses/#position
Okay?

Yes. Also, it is likely that USING CHARACTERS / USING OCTETS will be added in 2.10, but I'll create an issue for that.

Okay, we have agreed that I will go back to the text in
https://www.tarantool.io/en/doc/latest/reference/reference_sql/sql_statements_and_clauses/#position
So I will make this change in a separate commit.

doc/reference/reference_sql/sql_user_guide.rst

ImeevMA · 2021-10-20T16:39:52Z

doc/reference/reference_sql/sql_user_guide.rst

 See the description of NoSQL type :ref:`'number' <index-box_number>`.
+Support for arithmetic and built-in functions with NUMBERs was removed in Tarantool version 2.10.1.


Should we mention, that this is about "arithmetic" built-in functions? Some functions, such as typeof(), max(), least() and so on can work with NUMBERs.

Okay, I changed to built-in arithmetic functions

Sorry, but I can't see where this was changed.

ImeevMA · 2021-10-20T16:42:57Z

doc/reference/reference_sql/sql_user_guide.rst

 non-integer results.
 Example: ``5 / 2``, result = 2.

 ``%`` modulus (arithmetic)
-Divide second number into first number according to standard arithmetic rules.
+Divide second numeric into first numeric according to standard arithmetic rules.
 The result is the remainder.
 Example: ``17 % 5``, result = 2.

 ``<<`` shift left (arithmetic)


For % INTEGER could be of any sign, for example:

tarantool> box.execute([[SELECT -123 % 4;]]) --- - metadata: - name: COLUMN_1 type: integer rows: - [-3] ... tarantool> box.execute([[SELECT 123 % -4;]]) --- - metadata: - name: COLUMN_1 type: integer rows: - [3] ...

pgulutzan · 2021-10-21T18:43:38Z

@ImeevMA: Okay, I did a new commit.
I accepted 3 of your 4 suggestions, but am not sure what to do re POSITION, see my question.
I marked earlier comments "resolved" since you didn't say there were problems with how I handled them.
If you decide that we should continue to say that POSITION will not work with VARBINARY, then
I believe that your acceptance will take care of issue #2295 and issue #2296, but not issue #2298.

ImeevMA

Hey! Thanks for fixes! I can still see that 2 of my previous comments have not been fixed. Perhaps I am looking at the wrong version? I added one more non-essential comment to the content added in the previous commit. Sorry that I did not added it before. Also, it looks like I have no permission to resolve our conversations, so should I write something like “Resolved”?

ImeevMA · 2021-10-22T07:09:21Z

doc/reference/reference_sql/sql_statements_and_clauses.rst

+* When possible data types are any other scalar data type, SCALAR is default. |br|
+Example: ``box.execute([[SELECT TYPEOF(GREATEST(?,5);]]),{x})`` is 'scalar'. |br|
+* Otherwise, there is no default. |br|
+Example: ``box.execute([[SELECT TYPEOF(LIKELY(?);]]),{x})`` is unknown.


Perhaps I am overthinking, but to me the "unknown" sounds a little strange here. Maybe it's better to replace it with "the name of one of the primitive data types"?

ImeevMA · 2021-10-22T07:11:01Z

doc/reference/reference_sql/sql_user_guide.rst

 See the description of NoSQL type :ref:`'number' <index-box_number>`.
+Support for arithmetic and built-in functions with NUMBERs was removed in Tarantool version 2.10.1.


Sorry, but I can't see where this was changed.

ImeevMA · 2021-10-22T07:15:20Z

doc/reference/reference_sql/sql_user_guide.rst

 non-integer results.
 Example: ``5 / 2``, result = 2.

 ``%`` modulus (arithmetic)
-Divide second number into first number according to standard arithmetic rules.
+Divide second numeric into first numeric according to standard arithmetic rules.
 The result is the remainder.
 Example: ``17 % 5``, result = 2.

 ``<<`` shift left (arithmetic)


I still see

The result is the remainder. Starting with Tarantool version 2.10.1, operands must be positive INTEGER or UNSIGNED.

pgulutzan · 2021-12-01T20:47:39Z

@igormunkin: You self-requested a review two months ago. Are you still interested?

igormunkin

@pgulutzan, sorry for such delay: PR has been sunk in GitHub notifications traffic. The changes LGTM, except a couple of tiny nits. Thanks a lot!

doc/reference/reference_sql/sql_user_guide.rst

doc/reference/reference_sql/sql-features.rst

alexandra-mara

Hello @pgulutzan,
During the translation I encountered a few punctuation moments, as well as a couple of other suggestions, I would be happy if you could take a look at them

doc/reference/reference_sql/sql_user_guide.rst

alexandra-mara · 2021-12-10T11:12:07Z

doc/reference/reference_sql/sql_user_guide.rst

@@ -214,7 +214,7 @@ A literal has :ref:`data type = VARBINARY <sql_data_type_varbinary>`

 Here are four ways to put non-ASCII characters,such as the Greek letter α alpha,  in string literals: |br|
 First make sure that your shell program is set to accept characters as UTF-8. A simple way to check is |br|
-``SELECT hex('α');``
+``SELECT hex(cast('α' as VARBINARY));``
 If the result is CEB1 -- which is the hexadecimal value for the UTF-8 representation of α -- it is good. |br|
 (1) Simply enclose the character inside ``'...'``, |br|


@pgulutzan, this looks like a list, should we maybe format it like one? I mean, in ReStructuredText formatting.

That might be a good idea, but I think it should be considered for a different pull request. As you might have noticed, there are now some more data types, so after this is merged I will probably need to make a new pull request for "Even more behavior changes in SQL".

@patiencedaur should we make a task out of it and put it in backlog for now?

@alexandra-mara Yes please. Can you create an issue so we don't forget about it?

@patiencedaur: In the pull request #2523 you will see that I formatted the list with bullets.

doc/reference/reference_sql/sql_user_guide.rst

patiencedaur · 2021-12-14T12:20:54Z

doc/reference/reference_sql/sql_statements_and_clauses.rst

-   in effect use TYPEOF(item) not the defined data type.
+   from VARBINARY to a numeric, which is not sensible.
+#. The result data type of a primitive combination is sometimes SCALAR although we
+   in effect use the primitive data type not the defined data type.
   (Here we use the word "combination" in the way that the standard document


Hello @pgulutzan,
Is this the standard document you are talking about?

Not exactly, that's the technical corrigendum i.e. the list of corrections for SQL:2011. We care about SQL:2016 as mentioned earlier in the manual, and we care about the actual document not a corrigendum. However, there wasn't significant change between 2011 and 2016 for this or indeed for most of the features that Tarantool tries to support, so we can just say "standard document".

patiencedaur · 2021-12-15T10:43:08Z

doc/reference/reference_sql/sql_user_guide.rst

 for example ``'7' + '7'`` = 14.
 And for comparison, ``'7'`` = 7.
-This is called implicit casting. It is applicable for STRINGs and all numeric data types.
+This is called implicit casting. It was applicable for STRINGs and all numeric data types.
+Starting with Tarantool version 2.10, it is no longer supported.

 Limitations: (`Issue#2346 <https://github.com/tarantool/tarantool/issues/2346>`_) |br|
 * Some words, for example MATCH and REGEXP, are reserved but are not necessary for current or planned Tarantool versions |br|


By "MATCH and REGEXP are not necessary", do you mean that they can be easily replaced by other words? Can these words be used or do they cause an exception? Please clarify the "necessity" concept.

Since these words are reserved, they cannot be used except within quote marks, so yes their use would cause an error. Although it is a good idea to reserve a word if it is going to be used in a future Tarantool version, that is not the case here, or at least it wasn't the case when I wrote that and I know of no plan now. By the way, there are a few words that will be added to the reserved list soon.

patiencedaur · 2021-12-21T06:51:05Z

doc/reference/reference_sql/sql_user_guide.rst

+To represent "Inf" (infinity), write a real numeric outside the double-precision numeric range, for example 1E309.
+To represent "nan" (not a number), write an expression that does not result in a real numeric,


@pgulutzan By "real numeric", do you mean a number belonging to this mathematical set? That is, to represent "nan", you have to enter square root of minus one, for example?

True enough, although since we're talking about a restriction of the set, I prefer to think of the Wikipedia article about https://en.wikipedia.org/wiki/NaN. As described earlier, we avoid the English word "number" now except for the 'number' type, but perhaps in Russian translation there would be no such problem. Is this preventing you from approving the pull request? ~~~~

No, but this was preventing me from finishing the translation.

What made the matter more necessary now was that the NUMBER data type no longer had the typical characteristics that one associates with numbers. So in the initial comment I said
"
We will say "numeric" rather than "number" in some places. It's clumsy, but otherwise people will associate "number" with "NUMBER".
The alternative is to say, repeatedly, "number (but not NUMBER)".
"
Since no reviewer objected, I think it's considered okay.

Fixes gh-2206 Behavior changes in SQL

2f3aef3

pgulutzan requested a review from ImeevMA September 26, 2021 22:38

github-actions bot temporarily deployed to branch-pgulutzan-gh-2296 September 26, 2021 22:39 Inactive

github-actions bot temporarily deployed to translation-pgulutzan-gh-2296 September 26, 2021 22:39 Inactive

pgulutzan mentioned this pull request Sep 27, 2021

Define proper use of typeof() tarantool/tarantool#5956

Open

Fixes gh-2206 Behavior changes in SQL, second commit

7106777

github-actions bot temporarily deployed to branch-pgulutzan-gh-2296 September 27, 2021 21:57 Inactive

github-actions bot temporarily deployed to translation-pgulutzan-gh-2296 September 27, 2021 21:57 Inactive

Fixes gh-2206 Behavior changes in SQL, third commit

8b041ca

github-actions bot temporarily deployed to branch-pgulutzan-gh-2296 September 30, 2021 18:51 Inactive

github-actions bot temporarily deployed to translation-pgulutzan-gh-2296 September 30, 2021 18:54 Inactive

ImeevMA reviewed Oct 1, 2021

View reviewed changes

NickVolynkin linked an issue Oct 4, 2021 that may be closed by this pull request

Behavior changes in SQL #2296

Closed

Fixes gh-2206 Behavior changes in SQL, fourth commit

7b4d449

github-actions bot temporarily deployed to translation-pgulutzan-gh-2296 October 4, 2021 22:25 Inactive

github-actions bot temporarily deployed to branch-pgulutzan-gh-2296 October 4, 2021 22:25 Inactive

igormunkin self-requested a review October 5, 2021 12:47

pgulutzan requested a review from NickVolynkin October 11, 2021 18:03

ImeevMA reviewed Oct 20, 2021

View reviewed changes

Fixes gh-2206 Behavior changes in SQL, fifth commit

6e7b35c

github-actions bot temporarily deployed to translation-pgulutzan-gh-2296 October 21, 2021 18:28 Inactive

github-actions bot temporarily deployed to branch-pgulutzan-gh-2296 October 21, 2021 18:28 Inactive

ImeevMA reviewed Oct 22, 2021

View reviewed changes

Fixes gh-2206 Behavior changes in SQL, sixth commit

af60322

github-actions bot temporarily deployed to translation-pgulutzan-gh-2296 October 22, 2021 20:56 Inactive

github-actions bot temporarily deployed to branch-pgulutzan-gh-2296 October 22, 2021 20:56 Inactive

Fixes gh-2206 Behavior changes in SQL, sixth commit

5b667d1

NickVolynkin linked an issue Nov 8, 2021 that may be closed by this pull request

[2pt] QUOTE() and DOUBLE argument #2298

Closed

pgulutzan mentioned this pull request Nov 9, 2021

instr() is replaced with position() #2424

Closed

patiencedaur mentioned this pull request Nov 23, 2021

[3pt] Default types of SQL built-in functions #2455

Closed

patiencedaur assigned alexandra-mara Dec 3, 2021

igormunkin approved these changes Dec 3, 2021

View reviewed changes

doc/reference/reference_sql/sql_user_guide.rst Outdated Show resolved Hide resolved

doc/reference/reference_sql/sql-features.rst Show resolved Hide resolved

Fixes gh-2206 Behavior changes in SQL, eighth commit

7192767

github-actions bot temporarily deployed to branch-pgulutzan-gh-2296 December 3, 2021 22:08 Inactive

github-actions bot temporarily deployed to translation-pgulutzan-gh-2296 December 3, 2021 22:08 Inactive

pgulutzan mentioned this pull request Dec 3, 2021

Syntax for ARRAY in SQL #2475

Closed

alexandra-mara reviewed Dec 10, 2021

View reviewed changes

Fixes gh-2206 Behavior changes in SQL, ninth commit

d1f40b5

github-actions bot temporarily deployed to branch-pgulutzan-gh-2296 December 10, 2021 19:23 Inactive

github-actions bot temporarily deployed to translation-pgulutzan-gh-2296 December 10, 2021 19:24 Inactive

patiencedaur mentioned this pull request Dec 14, 2021

[8pt] Translate SQL reference #2496

Closed

6 tasks

patiencedaur reviewed Dec 14, 2021

View reviewed changes

patiencedaur reviewed Dec 15, 2021

View reviewed changes

Update translations

caf6c1d

github-actions bot temporarily deployed to translation-pgulutzan-gh-2296 December 20, 2021 13:13 Inactive

github-actions bot temporarily deployed to branch-pgulutzan-gh-2296 December 20, 2021 13:13 Inactive

Update translations

1cb42eb

github-actions bot temporarily deployed to translation-pgulutzan-gh-2296 December 20, 2021 13:59 Inactive

github-actions bot temporarily deployed to branch-pgulutzan-gh-2296 December 20, 2021 13:59 Inactive

patiencedaur reviewed Dec 21, 2021

View reviewed changes

Update translations

a87698b

github-actions bot temporarily deployed to translation-pgulutzan-gh-2296 December 22, 2021 06:41 Inactive

github-actions bot temporarily deployed to branch-pgulutzan-gh-2296 December 22, 2021 06:41 Inactive

patiencedaur merged commit cfb2984 into latest Dec 22, 2021

patiencedaur deleted the pgulutzan-gh-2296 branch December 22, 2021 07:08

patiencedaur mentioned this pull request Jan 19, 2022

Verify support for SQL:2016 features #690

Closed

		See the description of NoSQL type :ref:`'number' <index-box_number>`.
		Support for arithmetic and built-in functions with NUMBERs was removed in Tarantool version 2.10.1.

		To represent "Inf" (infinity), write a real numeric outside the double-precision numeric range, for example 1E309.
		To represent "nan" (not a number), write an expression that does not result in a real numeric,

Fixes gh-2206 Behavior changes in SQL #2350

Fixes gh-2206 Behavior changes in SQL #2350

Uh oh!

Conversation

pgulutzan commented Sep 26, 2021 • edited by NickVolynkin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ImeevMA left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ImeevMA Oct 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

NickVolynkin commented Oct 11, 2021

Uh oh!

pgulutzan commented Oct 11, 2021

Uh oh!

ImeevMA left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pgulutzan commented Oct 21, 2021

Uh oh!

ImeevMA left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pgulutzan commented Dec 1, 2021

Uh oh!

igormunkin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

alexandra-mara left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pgulutzan commented Sep 26, 2021 •

edited by NickVolynkin

Loading

ImeevMA Oct 1, 2021 •

edited

Loading

alexandra-mara left a comment •

edited

Loading

patiencedaur Dec 22, 2021 •

edited

Loading