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

Implement EXCEPT and REPLACE in select item #196

Conversation

apstndb
Copy link
Contributor

@apstndb apstndb commented Nov 8, 2024

This PR implements EXCEPT and REPLACE in SelectItem.

Note:

fixes #184

@apstndb apstndb changed the title Feature/select star except replace Implement EXCEPT and REPLACE in select item Nov 8, 2024
Replace token.Pos
Rparen token.Pos

Columns []*StarModifierReplaceItem
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO, we can reuse Alias here instead of introducing a new struct.

Copy link
Contributor Author

@apstndb apstndb Nov 13, 2024

Choose a reason for hiding this comment

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

Thank you for your comment. I agree that we can reuse Alias for it.

I've been avoiding using ast.Alias for a bit since ast.Alias is used for both of AS optional and AS required and it is not natural to implement parseAlias().
May be it is better to introduce parseAlias(requiredAs withAs).

Copy link
Contributor Author

@apstndb apstndb Nov 14, 2024

Choose a reason for hiding this comment

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

IMO, it is better to separate between expr AS alias and expr [AS] alias.
Currently they are not compatible with parseCommaSeparatedList unless we have a no-arg named parse* function.

Additionally, It seems StarModifierReplace is the first structure having expr AS name[, ...].
IMO, this is reason enough to have a separate StarModifierReplaceItem at this point.

Copy link
Collaborator

@makenowjust makenowjust left a comment

Choose a reason for hiding this comment

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

Thank you!

Comment on lines +684 to +685
// Note: The text/template notation escapes * to avoid normalize * to - by some formatters
// because the prefix * is ambiguous with bulletin list.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catch.

@makenowjust makenowjust merged commit 7eae73a into cloudspannerecosystem:main Dec 18, 2024
2 checks passed
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.

Support EXCEPT and REPLACE in SelectItem
2 participants