Skip to content

Fixes gh-2516 New SQL data types and function changes #2523

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
May 18, 2022

Conversation

pgulutzan
Copy link
Contributor

@pgulutzan pgulutzan commented Dec 24, 2021

Issue #2516 is intended as a fix for
Issue #2406 Literals for INTEGER, DECIMAL and DOUBLE
Issue #2439 SUBSTR() function
Issue #2440 Field type ANY in SQL
Issue #2455 Default types of SQL built-in functions
Issue #2461 Field type ARRAY in SQL
Issue #2462 Field type MAP in SQL
Issue #2475 Syntax for ARRAY in SQL
Probably reviewers will include @ImeevMA and @NickVolynkin.
Probably there will be several commits so consider this a first draft.
A possible controversial point is that I introduced arrays and maps in a section about literals rather than a later section about expressions -- putting them later would require some reorganizing.
UPDATE (2022-02-07): This is no longer a draft, so reviewer @ImeevMA can look at it whenever it's convenient.

@github-actions github-actions bot temporarily deployed to translation-pgulutzan-gh-2516 December 24, 2021 21:03 Inactive
@github-actions github-actions bot temporarily deployed to branch-pgulutzan-gh-2516 December 24, 2021 21:03 Inactive
@pgulutzan pgulutzan changed the title Fixes gh-2516 New SQL data types and function changes, first commit Fixes gh-2516 New SQL data types and function changes Dec 24, 2021
@veod32 veod32 requested a review from ImeevMA January 13, 2022 10:38
@ImeevMA
Copy link

ImeevMA commented Jan 13, 2022

Hi, @pgulutzan! I will review this PR. I also want to say that there will be a few more patches related to MAP and ARRAY. There will be a patch that adds a syntax for MAP values, patches that fix an error when MAP or ARRAY is used in ORDER BY/GROUP BY, and patches that fix some problems with using MAP and ARRAY in C user-defined functions. Also, it's possible that I will find more problems.

@pgulutzan
Copy link
Contributor Author

pgulutzan commented Jan 18, 2022

@ImeevMA: "A possible controversial point is that I introduced arrays and maps in a section about literals rather than a later section about expressions -- putting them later would require some reorganizing." What is your opinion?
By the way, I will make small style changes too, for example now we should if possible avoid saying "we" in the manual.
Update (a few days later): Never mind, I decided to try saying map and array literals are simple things that contain only a literal, and make a forward reference to map and array expressions which come later. The description of maps comes from from branch imeevma/gh-4763-syntax-for-map, not branch master (I don't see map expressions in branch master yet), and I added a mention that arrays can have integer indexes, which is purely a guess.

@patiencedaur patiencedaur self-requested a review January 20, 2022 05:14
@pgulutzan
Copy link
Contributor Author

pgulutzan commented Jan 21, 2022

@patiencedaur:

In sql_plus_lua.rst, examples that work in earlier versions will fail in Tarantool version 2.10.0.
So I'm adding comments in the code to tell users what to do if they have earlier versions.
For example:

.. code-block:: lua

box.schema.func.create('_DECODE',
   {language = 'LUA',
    returns = 'string',
    body = [[function (field, key)
             -- If Tarantool version < 2.10.0, replace next line with
             -- return require('msgpack').decode(field)[key]
             return field[key]
             end]],
    is_sandboxed = false,
    -- If Tarantool version < 2.10.0, replace next line with
    -- param_list = {'string', 'string'},
    param_list = {'map', 'string'},
    exports = {'LUA', 'SQL'},
    is_deterministic = true})

The comments should be translated for the Russian manual.
Alternative 1: Duplicate every example so there's an old and a new version.
Alternative 2: Put the instructions outside, and refer to line numbers.
If you like one of the alternatives,
or if you have a better idea,
we'll do whatever you prefer.

@patiencedaur
Copy link
Contributor

@pgulutzan Yes, let's do what you suggest. Thanks for letting me know.

@github-actions github-actions bot temporarily deployed to translation-pgulutzan-gh-2516 January 21, 2022 23:58 Inactive
@github-actions github-actions bot temporarily deployed to branch-pgulutzan-gh-2516 January 21, 2022 23:59 Inactive
@github-actions github-actions bot temporarily deployed to translation-pgulutzan-gh-2516 January 31, 2022 00:04 Inactive
@github-actions github-actions bot temporarily deployed to branch-pgulutzan-gh-2516 January 31, 2022 00:05 Inactive
@github-actions github-actions bot temporarily deployed to translation-pgulutzan-gh-2516 February 1, 2022 02:29 Inactive
@github-actions github-actions bot temporarily deployed to branch-pgulutzan-gh-2516 February 1, 2022 02:30 Inactive
@pgulutzan pgulutzan mentioned this pull request Feb 4, 2022
…imported recently-changed sql-features.rst and made small adjustments
@github-actions github-actions bot temporarily deployed to branch-pgulutzan-gh-2516 May 17, 2022 10:47 Inactive
@github-actions github-actions bot temporarily deployed to branch-pgulutzan-gh-2516 May 17, 2022 11:43 Inactive
Copy link

@ImeevMA ImeevMA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Thanks for the patch-set. I have a few comments below, but nothing major. I only reviewed the part about MAP and ARRAY.

* - MAP
- map
- (none)
- ``{'':''}``
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think {} can be considered smaller than {'':''}.

:ref:`column definitions <sql_column_def_data_type>` and the individual column values have
type ANY.
The difference between SCALAR and ANY is:
SCALAR columns may not contain MAP or ARRAY values, but ANY columns may contain them.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add that ANY values are not comparable (SCALAR values are comparable).

{ key : value } |br|
... for MAP expressions. |br|
Literal Examples: ``{'a':1}``, ``{ "column_1" : X'1234' }`` |br|
Non-literal Examples: ``{"a":"a"}``, ``{UUID(), (SELECT 1) + 1}`` |br|
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think here should be {UUID(): (SELECT 1) + 1}. Also, I think we should add some examples of MAP values where there is more than one key-value pairs, for example: {1:'a123', 'two':uuid()}.

(also called braces) ``{`` and ``}`` and contains a key for identification,
then a colon ``:``, then a value for what the key identifies.
The key data type must be INTEGER or STRING or UUID.
The value data type may be anything.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add that the value can also be a MAP, so MAPs can be nested (as was done for ARRAY).


.. _sql_array_index_expression:

ARRAY index expression: |br|
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should expand this part to include [] for MAP. Or it should be dropped, since the patch is currently in review: tarantool/tarantool#7140

@github-actions github-actions bot temporarily deployed to branch-pgulutzan-gh-2516 May 18, 2022 11:26 Inactive
@github-actions github-actions bot temporarily deployed to branch-pgulutzan-gh-2516 May 18, 2022 11:35 Inactive
@github-actions github-actions bot temporarily deployed to branch-pgulutzan-gh-2516 May 18, 2022 12:59 Inactive
@patiencedaur patiencedaur merged commit 40e8489 into latest May 18, 2022
@patiencedaur patiencedaur deleted the pgulutzan-gh-2516 branch May 18, 2022 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants