Skip to content

Conversation

@yittg
Copy link
Contributor

@yittg yittg commented Mar 24, 2022

While reading code about partition spec, i find some usages of magic number 999 which should be PARTITION_DATA_ID_START - 1 i think. Fix me if i misunderstand.

@rdblue would you help to review if you have time?

return UNPARTITIONED_SPEC;
}

public static int unpartitionedLastAssignedId() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this should be public.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

change it to package level.

@rdblue
Copy link
Contributor

rdblue commented Mar 28, 2022

Looks good overall. Thanks, @yittg!

@yittg yittg force-pushed the ref-magic-number branch from 55a006c to 7bc5013 Compare March 28, 2022 01:26
@yittg yittg requested a review from rdblue March 30, 2022 01:28
@yittg
Copy link
Contributor Author

yittg commented Apr 1, 2022

@rdblue would you like to merge this one?

@yittg yittg force-pushed the ref-magic-number branch from 7bc5013 to 8aa5521 Compare April 5, 2022 14:01
return UNPARTITIONED_SPEC;
}

private static int unpartitionedLastAssignedId() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is private now, @rdblue

@stevenzwu
Copy link
Contributor

There is a test failure that seems unrelated to this PR. cc @kbendick

org.apache.iceberg.flink.TestFlinkUpsert > testPrimaryKeyFieldsAtBeginningOfSchema[catalogName=testhadoop, baseNamespace=default, format=PARQUET, isStreaming=true] FAILED
    java.lang.AssertionError: 
    Expecting:
      [+I[bbb, 2022-03-01, 3], +I[aaa, 2022-03-01, 1]]
    to contain exactly in any order:
      [+I[aaa, 2022-03-01, 2], +I[bbb, 2022-03-01, 3]]
    elements not found:
      [+I[aaa, 2022-03-01, 2]]
    and elements not expected:
      [+I[aaa, 2022-03-01, 1]]
        at org.apache.iceberg.flink.TestHelpers.assertRows(TestHelpers.java:137)
        at org.apache.iceberg.flink.TestFlinkUpsert.testPrimaryKeyFieldsAtBeginningOfSchema(TestFlinkUpsert.java:[219](https://github.com/apache/iceberg/runs/5834902185?check_suite_focus=true#step:6:219))

@yittg
Copy link
Contributor Author

yittg commented Apr 6, 2022

Thanks @stevenzwu .

And shall we open an issue to track this fragile test case, @kbendick .
#4515 is exactly this issue

@yittg yittg force-pushed the ref-magic-number branch from 8aa5521 to f985fcf Compare April 12, 2022 01:02
@rdblue rdblue merged commit 18a593c into apache:master Apr 15, 2022
@yittg yittg deleted the ref-magic-number branch April 16, 2022 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants