feat: parse DataType List, ListView, LargeList, LargeListView, FixedSizeList#8649
feat: parse DataType List, ListView, LargeList, LargeListView, FixedSizeList#8649alamb merged 15 commits intoapache:mainfrom
List, ListView, LargeList, LargeListView, FixedSizeList#8649Conversation
dqkqd
force-pushed
the
support-parse-list
branch
from
5826fb5 to
e25e4a1
Compare
dqkqd
changed the title
nullability and field for DataType::ListList, ListView, LargeList, LargeListView, FixedSizeList
|
|
||
| "nullable" => Token::Nullable, | ||
| "field" => Token::Field, | ||
| "x" => Token::X, |
There was a problem hiding this comment.
I cannot find a better name for x.
Using Time seems confuse with time related data types.
alamb
changed the title
List, ListView, LargeList, LargeListView, FixedSizeListList, ListView, LargeList, LargeListView, FixedSizeList
Co-authored-by: Andrew Lamb <[email protected]>
dqkqd
force-pushed
the
support-parse-list
branch
from
bc35472 to
b6d502c
Compare
|
Thank you @alamb for reviewing. |
|
Thanks again @dqkqd 🙏 |
|
While testing the next 57 release, I believe this PR has introduced several behavior changes / regresssions:
I have a fix for #8880 but ran out of time to look into #8883 more carefully . Does anyone have time to help check it out to unblock the release? |
# Which issue does this PR close? - Closes #8880 # Rationale for this change - #8649 change the default display for List and LargeList and FixedSizeList, and changed the parsing code to match. However, to maintain backwards compatibility we also need to support the old syntax too # What changes are included in this PR? WIP # Are these changes tested? WIP # Are there any user-facing changes?
|
Update here is that in order to maintain backwards compatibility we changed the behavior in So that For example
|
3 participants
Which issue does this PR close?
parse_data_typeforList,ListView,LargeList,LargeListView,FixedSizeList,Union,Map,RunEndCoded. #8648.This PR only implements for list types to make review easier.
Rationale for this change
The format for
DataType::Listincludes:List(Int64): list not nullable.List(nullable Int64): list nullable.List(nullable Int64, field: 'foo'): list nullable with field.List(nullable Int64, metadata: {"foo1": "value1"}): list with metadata.(... The list goes on for
ListView,LargeList,LargeListView,FixedSizeList)parse_data_typecannot (or incorrectly) work on those data types listed above.What changes are included in this PR?
Token::...to support newDisplayformat for list types introduced in ImproveDisplayforDataType#8351(e.g.
FixedSizeList(5 x nullable Int64, field: 'foo').fn nullableto check whether nested data type is nullable.parse_single_quoted_stringandparse_list_field_nameto handlefield: 'foo'.Are these changes tested?
Yes. Added round trip tests.
Are there any user-facing changes?
Yes. This is related to #8351