Skip to content

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

Merged
reuvenlax merged 4 commits intoapache:masterfrom
reuvenlax:bytebuddy_recursive_types
Dec 5, 2019
Merged

[BEAM-7274] Support recursive type transformation in ByteBuddyUtils#10278
reuvenlax merged 4 commits intoapache:masterfrom
reuvenlax:bytebuddy_recursive_types

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 force-pushed the bytebuddy_recursive_types branch from d97e213 to 2d1bcc0 Compare December 5, 2019 18:40
@reuvenlax reuvenlax changed the title Bytebuddy recursive types [BEAM-7274] Support recursive type transformation in ByteBuddyUtils Dec 5, 2019
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
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.

2 participants