fix(json): remove artificial arg limit on JSONArray [CLAUDE]#7559
Open
maxa-jonathan-hallee wants to merge 1 commit intotobymao:mainfrom
Open
fix(json): remove artificial arg limit on JSONArray [CLAUDE]#7559maxa-jonathan-hallee wants to merge 1 commit intotobymao:mainfrom
maxa-jonathan-hallee wants to merge 1 commit intotobymao:mainfrom
Conversation
JSONArray enforced a 4-argument maximum because is_var_len_args was
False and arg_types had only 4 keys. DuckDB's JSON_ARRAY is variadic
and accepts any number of arguments, so queries like
JSON_ARRAY('a', 'b', 'c', 'd', 'e') failed at parse time.
Setting is_var_len_args = True skips the arg count validation while
preserving existing parsing behavior across all dialects.
Ref: https://duckdb.org/docs/current/data/json/creating_json
Collaborator
georgesittas
left a comment
georgesittas
left a comment
There was a problem hiding this comment.
Hey @maxa-jonathan-hallee, thanks for the PR. This is almost there, but needs one change: DuckDB needs to override the instantiation of JSON_ARRAY in the parser so that args don't spill over to strict. You can see this is what happens now if you parse a function call with more than 4 arguments. Instead, you need to make sure args are set to expressions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
JSONArrayenforced a 4-argument maximum becauseis_var_len_argswasFalseandarg_typeshad only 4 keysJSON_ARRAYis variadic and accepts any number of arguments (docs), so queries likeJSON_ARRAY('a', 'b', 'c', 'd', 'e')failed at parse timeis_var_len_args = Trueskips the arg count validation while preserving existing parsing behavior across all dialectsTo the best of my understanding this isn't really a problem for other dialect parsers
Test plan
validate_alltest with 5 args covering DuckDB round-trip and Snowflake transpilation