Skip to content

builtins: implement parse_ident function#93300

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
rafiss:parse-ident
Dec 14, 2022
Merged

builtins: implement parse_ident function#93300
craig[bot] merged 1 commit intocockroachdb:masterfrom
rafiss:parse-ident

Conversation

@rafiss
Copy link
Copy Markdown
Collaborator

@rafiss rafiss commented Dec 9, 2022

fixes #93159

The implementation was adapted from the Postgres one.

Release note (sql change): Implemented the parse_ident builtin. The function splits a qualified identifier into an array of identifiers, removing any quoting of individual identifiers. By default, extra characters after the last identifier are considered an error; but if the second parameter is false, then such extra characters are ignored.

@rafiss rafiss requested a review from a team December 9, 2022 02:44
@cockroach-teamcity
Copy link
Copy Markdown
Member

cockroach-teamcity commented Dec 9, 2022

This change is Reviewable

@rafiss rafiss requested a review from ZhouXing19 December 9, 2022 16:04
@ZhouXing19
Copy link
Copy Markdown
Collaborator

ZhouXing19 commented Dec 14, 2022

:lgtm_strong: ! I just have a small question about the test.

ZhouXing19
ZhouXing19 approved these changes Dec 14, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@ZhouXing19 ZhouXing19 left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @rafiss)

pkg/sql/sem/builtins/parse_ident_builtin_test.go line 1 at r1 (raw file):

// Copyright 2022 The Cockroach Authors.

Just curious why not include these tests in the data-driven logic test as well.

rafiss
rafiss commented Dec 14, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator Author

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

tftr!

bors r=ZhouXing19

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @ZhouXing19)

pkg/sql/sem/builtins/parse_ident_builtin_test.go line 1 at r1 (raw file):

Previously, ZhouXing19 (Jane Xing) wrote…

Just curious why not include these tests in the data-driven logic test as well.

this is just a guess, but i don't think we really need to license any of our testing code. i think the .go test files just have them because our linter checks all .go files

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Dec 14, 2022

Build failed (retrying...):

@renatolabs
Copy link
Copy Markdown

renatolabs commented Dec 14, 2022 

[Run local roachtests] compilepkg: nogo: errors found by nogo during build-time code analysis:
[Run local roachtests] pkg/sql/sem/builtins/builtins.go:3176:32: github.com/cockroachdb/cockroach/pkg/sql/sem/tree.ParamType struct literal uses unkeyed fields (composites)
[Run local roachtests] pkg/sql/sem/builtins/builtins.go:3199:5: github.com/cockroachdb/cockroach/pkg/sql/sem/tree.ParamType struct literal uses unkeyed fields (composites)
[Run local roachtests] pkg/sql/sem/builtins/builtins.go:3200:5: github.com/cockroachdb/cockroach/pkg/sql/sem/tree.ParamType struct literal uses unkeyed fields (composites)

bors r-

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Dec 14, 2022

Canceled.

@rafiss
builtins: implement parse_ident function
5386a66 
The implementation was adapted from the Postgres one.

Release note (sql change): Implemented the parse_ident builtin.
The function splits a qualified identifier into an array of identifiers,
removing any quoting of individual identifiers. By default,
extra characters after the last identifier are considered an error;
but if the second parameter is false, then such extra characters are ignored.
@rafiss
Copy link
Copy Markdown
Collaborator Author

rafiss commented Dec 14, 2022

bors r=ZhouXing19

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Dec 14, 2022

Build succeeded:

@craig craig bot merged commit dbea195 into cockroachdb:master Dec 14, 2022
@blathers-crl
Copy link
Copy Markdown

blathers-crl bot commented Dec 14, 2022

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.

error creating merge commit from 5386a66 to blathers/backport-release-22.2-93300: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 22.2.x failed. See errors above.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@rafiss rafiss mentioned this pull request Dec 14, 2022
Merged
@cockroach-teamcity cockroach-teamcity mentioned this pull request Dec 15, 2022
Closed
@rafiss rafiss deleted the parse-ident branch December 16, 2022 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ZhouXing19 ZhouXing19 ZhouXing19 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

builtins: implement parse_ident function

4 participants

@rafiss @cockroach-teamcity @ZhouXing19 @renatolabs