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.

Sign up for GitHub

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

Jump to bottom

More test coverage for Iceberg partitioning transforms #12865

Merged
merged 4 commits into from
Jun 20, 2022
Merged

More test coverage for Iceberg partitioning transforms #12865

merged 4 commits into from
Jun 20, 2022

Conversation

findepi
Copy link
Member

@findepi findepi commented Jun 15, 2022

Extracted from #12795

@findepi
Test Iceberg partitioning transforms with NULL values
3d4c25f 
Add NULL values in Iceberg partitioning transforms with NULL values.
NULL values should be partitioned separately from non-NULL values. The
`void` transform is an exception, and it is already tested with NULL
values.
@cla-bot cla-bot bot added the cla-signed label Jun 15, 2022
@findepi findepi mentioned this pull request Jun 15, 2022
Merged
@findepi findepi added test no-release-notes This pull request does not require release notes entry labels Jun 15, 2022
@findepi
Copy link
Member Author

findepi commented Jun 20, 2022

Failures related

Error:    TestIcebergOrcConnectorTest>BaseIcebergConnectorTest.testYearTransformTimestampWithTimeZone:1745 Plan does not match, expected [
- node(OutputNode)
    - node(ExchangeNode)
        - node(TableScanNode)
] but found [
Output[d, b]
│   Layout: [d:timestamp(6) with time zone, b:integer]
└─ RemoteExchange[GATHER]
   │   Layout: [d:timestamp(6) with time zone, b:integer]
   └─ ScanFilter[table = iceberg:tpch.test_year_transform_timestamptz$data@4201418955623477458, filterPredicate = (NOT ("d" IS NULL))]
          Layout: [d:timestamp(6) with time zone, b:integer]
          b := 2:b:integer
          d := 1:d:timestamp(6) with time zone
]
Error:    TestIcebergParquetConnectorTest>BaseIcebergConnectorTest.testYearTransformTimestampWithTimeZone:1745 Plan does not match, expected [
- node(OutputNode)
    - node(ExchangeNode)
        - node(TableScanNode)
] but found [
Output[d, b]
│   Layout: [d:timestamp(6) with time zone, b:integer]
└─ RemoteExchange[GATHER]
   │   Layout: [d:timestamp(6) with time zone, b:integer]
   └─ ScanFilter[table = iceberg:tpch.test_year_transform_timestamptz$data@837544948429595466, filterPredicate = (NOT ("d" IS NULL))]
          Layout: [d:timestamp(6) with time zone, b:integer]
          b := 2:b:integer
          d := 1:d:timestamp(6) with time zone

@findepi
Test Iceberg transforms with timestamp with time zone
aa1590b 
This adds test for `day`, `month` and `year` partitioning transforms
over `tiemstamp with time zone`. They are similar to existing tests for
`timestamp` type.
@findepi
Test Iceberg OPTIMIZE with time partitioned table
de997be
@findepi
Remove redundant code spacing
6d45903 
Tests follow common pattern, and some have spacing and some do not.
@findepi findepi force-pushed the findepi/iceberg-more-tests branch from 35c7d00 to 6d45903 Compare June 20, 2022 10:31
@findepi findepi merged commit 5ab1f58 into trinodb:master Jun 20, 2022
@findepi findepi deleted the findepi/iceberg-more-tests branch June 20, 2022 13:55
@github-actions github-actions bot added this to the 387 milestone Jun 20, 2022
@colebow colebow mentioned this pull request Jun 21, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ebyhr ebyhr ebyhr approved these changes

@erichwang erichwang Awaiting requested review from erichwang

@losipiuk losipiuk Awaiting requested review from losipiuk

@alexjo2144 alexjo2144 Awaiting requested review from alexjo2144

@hashhar hashhar Awaiting requested review from hashhar

Assignees
No one assigned
Labels
cla-signed no-release-notes This pull request does not require release notes entry test
Milestone
387
Development

Successfully merging this pull request may close these issues.
2 participants
@findepi @ebyhr