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

[BEAM-7274] Support recursive type transformation in ByteBuddyUtils #10278

Merged
merged 4 commits into from
Dec 5, 2019

Conversation

reuvenlax
Copy link
Contributor

@reuvenlax reuvenlax commented Dec 3, 2019

This was uncovered by the attempt to add protocol buffer support to schemas. This PR refactors the schema utils to convert parameter types (e.g. for List to convert T as well). As part of this PR, Row now represents ARRAY types as a Collection instead of as a List.

R: @alexvanboxel

@reuvenlax reuvenlax changed the title Bytebuddy recursive types [BEAM-7274] Support recursive type transformation in ByteBuddyUtils Dec 5, 2019

/** A set of reflection helper methods. */
public class ReflectUtils {
static class ClassWithSchema {
/** Represents a class and a schema. */
public static class ClassWithSchema {
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume it has a reason that they are now public? (was package protected)

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's actually public because we need to use this in the protobuf schema provider which is in a different package (and in general, you should be able to put schema providers in other packages).

@alexvanboxel
Copy link
Contributor

In general this LGTM. I see optimizations for the List vs Collection as List will be the most common use case. If I remember correctly the instanceof has minimal impact.

@reuvenlax
Copy link
Contributor Author

run sql postcommit

@reuvenlax reuvenlax merged commit f6f809f into apache:master Dec 5, 2019
11moon11 pushed a commit to 11moon11/beam that referenced this pull request Dec 12, 2019
dpcollins-google pushed a commit to dpcollins-google/beam that referenced this pull request Dec 20, 2019
JozoVilcek pushed a commit to JozoVilcek/beam that referenced this pull request Feb 21, 2020
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.

None yet

2 participants