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

API: StructProjection returns a null Projection object when the nested struct is null #7507

Closed
stevenzwu opened this issue May 3, 2023 · 6 comments
Labels

Comments

@stevenzwu
Copy link
Contributor

stevenzwu commented May 3, 2023

Today, if the nested struct is null, StructProjection returns a nested StructProjection object wraps the null struct value. It is probably simpler if a null projection object is returned instead. Then the projection classes like (StructProjection or Flink RowDataProjection) don't need to handle null wrapped struct.

See more context in #7493 (comment).

@rdblue @aokolnychyi @RussellSpitzer @Reo-LEI

@stevenzwu stevenzwu changed the title Core: StructProjection returns a null Projection object when the nested struct is null API: StructProjection returns a null Projection object when the nested struct is null May 3, 2023
@stevenzwu
Copy link
Contributor Author

stevenzwu commented May 4, 2023

if we have a schema like this

Schema schema =
        new Schema(
            Types.NestedField.required(0, "id", Types.LongType.get()),
            Types.NestedField.optional(
                3,
                "location",
                Types.StructType.of(
                    Types.NestedField.required(1, "lat", Types.FloatType.get()),
                    Types.NestedField.required(2, "long", Types.FloatType.get()))));

when the nested location struct is null

GenericRowData rowDataNullStruct = GenericRowData.of(1L, null);

the StructProjection or Flink RowDataProjection would create a projection object wrapping a null struct value. I was proposing that it should just return a null projection object directly.

@stevenzwu
Copy link
Contributor Author

before: StructProjection(1, StructProjection(null))
after: StructProjection(1, null)

@stevenzwu
Copy link
Contributor Author

then the Projection class doesn't have to deal with null wrapped value. e.g.

 public <T> T get(int pos, Class<T> javaClass) {
    if (struct == null) {
      // Return a null struct when projecting a nested required field from an optional struct.
      // See more details in issue #2738.
      return null;
    }

it also mirroring the original struct more faithfully.

Struct(1, null) -> StructProjection(1, null)    |   instead of StructProjection(1, StructProjection(null))

Copy link

github-actions bot commented Nov 1, 2023

This issue has been automatically marked as stale because it has been open for 180 days with no activity. It will be closed in next 14 days if no further activity occurs. To permanently prevent this issue from being considered stale, add the label 'not-stale', but commenting on the issue is preferred when possible.

@github-actions github-actions bot added the stale label Nov 1, 2023
Copy link

This issue has been closed because it has not received any activity in the last 14 days since being marked as 'stale'

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 17, 2023
@nastra
Copy link
Contributor

nastra commented Nov 17, 2023

@stevenzwu does this still apply? If so, then we should re-open and add the not-stale tag so that the issue doesn't get automatically closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants