Skip to content
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

Quote row type field names #1963

Merged
merged 5 commits into from Nov 8, 2019

Conversation

@martint
Copy link
Member

martint commented Nov 5, 2019

Since row type field names can match reserved SQL keywords, names
need to be quoted when rendering them.

Fixes #1962

@cla-bot cla-bot bot added the cla-signed label Nov 5, 2019
@martint martint force-pushed the martint:row-reserved-name branch 4 times, most recently from 65e1afc to 3fab865 Nov 5, 2019
@@ -319,6 +319,11 @@ public void testRowSubscript()

// Row subscript in join condition
assertQuery("SELECT n.name, r.name FROM nation n JOIN region r ON ROW (n.name, n.regionkey)[2] = ROW (r.name, r.regionkey)[2] ORDER BY n.name LIMIT 1", "VALUES ('ALGERIA', 'AFRICA')");

// Subscript over field named after reserved keyword
assertQuery(

This comment has been minimized.

Copy link
@kokosing

kokosing Nov 6, 2019

Member

I think we should have a dedicated test for planner (TestPlanner?). There is no point to run this test for all the connectors.

This comment has been minimized.

Copy link
@dain

dain Nov 6, 2019

Member

IIRC the issue occurs when distributing plan fragments to workers, so this must run in distributed mode.

@martint martint force-pushed the martint:row-reserved-name branch from 3fab865 to b5a30ab Nov 6, 2019
@dain
dain approved these changes Nov 6, 2019
@@ -319,6 +319,11 @@ public void testRowSubscript()

// Row subscript in join condition
assertQuery("SELECT n.name, r.name FROM nation n JOIN region r ON ROW (n.name, n.regionkey)[2] = ROW (r.name, r.regionkey)[2] ORDER BY n.name LIMIT 1", "VALUES ('ALGERIA', 'AFRICA')");

// Subscript over field named after reserved keyword
assertQuery(

This comment has been minimized.

Copy link
@dain

dain Nov 6, 2019

Member

IIRC the issue occurs when distributing plan fragments to workers, so this must run in distributed mode.

@martint martint force-pushed the martint:row-reserved-name branch from b5a30ab to f502d5a Nov 6, 2019
@findepi

This comment has been minimized.

Copy link
Member

findepi commented Nov 7, 2019

IIRC the issue occurs when distributing plan fragments to workers, so this must run in distributed mode.

TestTpchDistributedQueries?

it already is used to test distribution-related things, eg testTooManyStages

martint added 5 commits Nov 5, 2019
Names can be reserved SQL keywords, so we need to quote them
to avoid issues when deserializing plans on workers.
TypeSignature is used for encoding the identity of a type, which
is then parsed (for now) using the SQL parser. Since some fields
names might collide with SQL reserved words, we need to quote them.
@martint martint force-pushed the martint:row-reserved-name branch from f502d5a to 9adf159 Nov 8, 2019
@martint martint merged commit 564ae8a into prestosql:master Nov 8, 2019
2 checks passed
2 checks passed
Travis CI - Pull Request Build Passed
Details
verification/cla-signed
Details
@martint martint referenced this pull request Nov 8, 2019
3 of 6 tasks complete
@martint martint added this to the 325 milestone Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.