GenericDialect: support colon operator for JsonAccess#2124
GenericDialect: support colon operator for JsonAccess#2124iffyio merged 1 commit intoapache:mainfrom
Conversation
Samyak2
commented
Dec 4, 2025
Samyak2
commented
- Port JsonAccess colon operator from Snowflake to Generic dialect
- This will be used in variant data type support in Datafusion
- see discussion in epic: Initial implementation of this crate datafusion-contrib/datafusion-variant#2
Samyak2
force-pushed
the
generic-json-access
branch
2 times, most recently
from
2a3a6c0 to
5b6007b
Compare
|
I will fix the CI in a bit - sorry did not notice that! |
Samyak2
force-pushed
the
generic-json-access
branch
from
5b6007b to
4ae25fd
Compare
|
CI should pass now. There were some changes in main that I had not rebased on. |
| let dialects = TestedDialects::new(vec![ | ||
| Box::new(GenericDialect {}), | ||
| Box::new(SnowflakeDialect {}), | ||
| ]); |
There was a problem hiding this comment.
should this be all dialects instead? we don't seem to have special handling for generic and snowflake
There was a problem hiding this comment.
We actually do have special handling for SnowflakeDialect and GenericDialect:
datafusion-sqlparser-rs/src/parser/mod.rs
Line 3814 in ce74e7f
(this is in main currently, not a change in this PR)
From what I know, only Snowflake and Databricks support this syntax. So I can expand the test to include Databricks as well. But I don't think it would make sense to run this test on dialects that don't support this syntax. What do you think?
There was a problem hiding this comment.
I have added Databricks to the list of dialects to test for.
| } else { | ||
| Some(self.parse_expr()?) | ||
| // parse expr until we hit a colon (or any token with lower precedence) | ||
| Some(self.parse_subexpr(self.dialect.prec_value(Precedence::Colon))?) |
There was a problem hiding this comment.
for the changes to the subscript behavior, we don't seem to have any new tests to accompany them?
There was a problem hiding this comment.
These changes actually came from a failing test, but I will add some more tests to explicitly look for this behavior.
There was a problem hiding this comment.
I have added some parse_array_subscript tests. I have verified that these catch the fixes made by these changes in subscript behavior.
- Port JsonAccess colon operator from Snowflake to Generic dialect - This will be used in variant data type support in Datafusion - see discussion in datafusion-contrib/datafusion-variant#2
Samyak2
force-pushed
the
generic-json-access
branch
from
4ae25fd to
f365029
Compare
- Fixes apache#2204 - Make map_field parsing aware of Colon token. - This regression was likely introduced in apache#2124 - Added some tests for this. - Not sure how the existing tests are passing, but I have verified that the new tests catch this bug (they fail without the fix).
3 participants
