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.

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

Revert "do not merge: always force parquet format in cdc tests" #103928

Merged
merged 2 commits into from
Jun 1, 2023

Conversation

jayshrivastava
Copy link
Contributor

@jayshrivastava jayshrivastava commented May 25, 2023

Revert "do not merge: always force parquet format in cdc tests" #103928

This reverts commit 63eccd4.

Release note: None
Epic: None


changefeedccl: refactor parquet column iterators

Previously, some tests would fail due to invalid conversion
from parquet to JSON in testing. These failures showed two
underlying issues with how columns were written:

  • in tests watching a column family not containing the primary
    key of the table, we would not write the primary key to
    the parquet file
  • in tests where the primary key was defined with a particular
    order which is different than the table, the parquet testing
    code would not order the keys correctly

This change fixes the two above issues by (a) using the correct
iterators on cdcevent.Row and writing more verbose
metadata to the parquet file to use in testing.

Epic: None
Release note: None

…sible"

This reverts commit 63eccd4.

Release note: None
Epic: None
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@jayshrivastava jayshrivastava changed the title Revert "do not merge: always force parquet format in cdc tests if pos… Revert "do not merge: always force parquet format in cdc tests" May 25, 2023
@jayshrivastava jayshrivastava marked this pull request as ready for review May 25, 2023 21:15
@jayshrivastava jayshrivastava requested a review from a team as a code owner May 25, 2023 21:15
@jayshrivastava jayshrivastava mentioned this pull request May 25, 2023
13 tasks
@jayshrivastava jayshrivastava force-pushed the revert-parquet branch 2 times, most recently from 1b6c82d to 4f1b514 Compare May 31, 2023 15:33
Previously, some tests would fail due to invalid conversion
from parquet to JSON in testing. These failures showed two
underlying issues with how columns were written:
- in tests watching a column family not containing the primary
  key of the table, we would not write the primary key to
  the parquet file
- in tests where the primary key was defined with a particular
  order which is different than the table, the parquet testing
  code would not order the keys correctly

This change fixes the two above issues by (a) using the correct
iterators on `cdcevent.Row` and writing more verbose
metadata to the parquet file to use in testing.

Epic: None
Release note: None
@jayshrivastava jayshrivastava force-pushed the revert-parquet branch 2 times, most recently from df3f06b to 875cbfa Compare May 31, 2023 22:54
@jayshrivastava
Copy link
Contributor Author

bors r=HonoreDB

@craig
Copy link
Contributor

craig bot commented Jun 1, 2023

Build succeeded:

@craig craig bot merged commit 294edf6 into cockroachdb:master Jun 1, 2023
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