Skip to content

Conversation

@AbhishekASLK
Copy link
Contributor

Previously, the parser defaulted to the strict ANSI implementation, causing a ParseError when encountering the
comma-separated variant.

SQL:

SELECT overlay('Spark SQL', 'ANSI ', 7, 0)

Databricks Docs

Copy link
Collaborator

@VaggelisD VaggelisD 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 for the contribution @AbhishekASLK! Two minor comments to consider and its fine to merge

from_=self._match_text_seq("FROM") and self._parse_bitwise(),
for_=self._match_text_seq("FOR") and self._parse_bitwise(),
)

Copy link
Collaborator

@VaggelisD VaggelisD Dec 8, 2025

Choose a reason for hiding this comment

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

We can probably simplify this as:

        def _parse_overlay(self) -> exp.Overlay:
            def _parse_overlay_arg(text: str) -> t.Optional[exp.Expression]:
               return (self._match(TokenType.COMMA) or self._match_text_seq(text)) and self._parse_bitwise()
             
            return self.expression(
                exp.Overlay,
                this=self._parse_bitwise(),
                expression=_parse_overlay_arg("PLACING"),
                from_=_parse_overlay_arg("FROM"),
                for_=_parse_overlay_arg("FOR"),
            )

Choose a reason for hiding this comment

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

Thanks to simplify this, will modify it as suggested.

Comment on lines +430 to +435
self.validate_all(
"SELECT OVERLAY('Spark SQL', 'ANSI ', 7, 0)",
write={
"databricks": "SELECT OVERLAY('Spark SQL' PLACING 'ANSI ' FROM 7 FOR 0)",
},
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

validate_all should be used when we also want to check transpilations between other dialects; When testing the same dialect with expected changes we should still use validate_identity:

Suggested change
self.validate_all(
"SELECT OVERLAY('Spark SQL', 'ANSI ', 7, 0)",
write={
"databricks": "SELECT OVERLAY('Spark SQL' PLACING 'ANSI ' FROM 7 FOR 0)",
},
)
self.validate_identity(
"SELECT OVERLAY('Spark SQL', 'ANSI ', 7, 0)",
"SELECT OVERLAY('Spark SQL' PLACING 'ANSI ' FROM 7 FOR 0)"
)

Choose a reason for hiding this comment

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

Okay, got it!

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