Skip to content

Conversation

@mingmxu
Copy link

@mingmxu mingmxu commented Jun 23, 2017

R: @takidau
As discussed in #3372, here's the part to use fixed table name PCOLLECTION in BeamSql.simpleQuery.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 1f61204 on XuMingmin:BEAM-2503 into ** on apache:DSL_SQL**.

Copy link
Contributor

@takidau takidau left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you! I'll go ahead and merge.

public PCollection<BeamSqlRow> expand(PCollection<BeamSqlRow> input) {
// public SimpleQueryTransform withUdf(String udfName){
// throw new UnsupportedOperationException("Pending for UDF support");
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

Best not to check in commented-out code. Since I have no other requests for changes, I'll remove as part of merging.

@asfgit asfgit merged commit 1f61204 into apache:DSL_SQL Jun 27, 2017
asfgit pushed a commit that referenced this pull request Jun 27, 2017
@takidau
Copy link
Contributor

takidau commented Jun 27, 2017

Merged. Forgot to remove the commented out code. If it's not getting uncommented the next time you edit this file, let's please remove it then. Feel free to close this PR.

@mingmxu
Copy link
Author

mingmxu commented Jun 27, 2017

Thanks @takidau .

The commented lines are pending to fix in another task, would handle it in BEAM-2520.

@mingmxu mingmxu deleted the BEAM-2503 branch June 27, 2017 07:22
@takidau
Copy link
Contributor

takidau commented Jun 28, 2017

For a feature branch I think that's probably okay, but I wouldn't want to leave commented-out code lying around on master. Thanks! Feel free to close.

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.

4 participants