Skip to content

Conversation

@wypoon
Copy link
Contributor

@wypoon wypoon commented Oct 26, 2022

In #6041, I ended up with a slightly different version of TestSparkReaderDeletes for v3.2 than in v3.3 because I created the PR before #6026 (the original PR, for v3.3) was merged and I then made some tweaks to TestSparkReaderDeletes for #6026. Here I align the version of TestSparkReaderDeletes in v3.2 with the version in v3.3. For future backports, it is easier when the 3.2 code base is the same as the 3.3 code base where there is no reason to be different.

@github-actions github-actions bot added the spark label Oct 26, 2022
@wypoon
Copy link
Contributor Author

wypoon commented Oct 26, 2022

@flyrain sorry to trouble you again -- can you please review this? There were some simple improvements to TestSparkReaderDeletes in the v3.3 version that are missing from the v3.2 version. It's not a big deal, but would be nice to keep the code the same.

Assume.assumeTrue(format.equals("parquet"));

String tblName = "test_multiple_row_groups";
String tblName = "test3";
Copy link
Contributor

@flyrain flyrain Oct 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kinda prefer meaningful name, like table_with_multiple_row_groups, maybe we can change the 3.3 with a more meaningful name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I changed it to test3 in the original (3.3) PR was that in the base class, the tables were test and test2, so I used test3 for this new table to be consistent!

Copy link
Contributor

@flyrain flyrain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@flyrain flyrain merged commit 96f0325 into apache:master Oct 28, 2022
@flyrain
Copy link
Contributor

flyrain commented Oct 28, 2022

Merged. Thanks @wypoon!

sunchao pushed a commit to sunchao/iceberg that referenced this pull request May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants