[GitHub] flink pull request #6267: [FLINK-5750] Incorrect parse of brackets inside VA...

2018-07-09 Thread AlexanderKoltsov 
Github user AlexanderKoltsov closed the pull request at:

https://github.com/apache/flink/pull/6267


---

[GitHub] flink pull request #6267: [FLINK-5750] Incorrect parse of brackets inside VA...

2018-07-05 Thread fhueske 
Github user fhueske commented on a diff in the pull request:

https://github.com/apache/flink/pull/6267#discussion_r200478425
  
--- Diff: 
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/plan/nodes/dataset/DataSetUnion.scala
 ---
@@ -36,22 +39,21 @@ import scala.collection.JavaConverters._
 class DataSetUnion(
--- End diff --

We need the same fix for `DataStreamUnion`


---

[GitHub] flink pull request #6267: [FLINK-5750] Incorrect parse of brackets inside VA...

2018-07-05 Thread fhueske 
Github user fhueske commented on a diff in the pull request:

https://github.com/apache/flink/pull/6267#discussion_r200478336
  
--- Diff: 
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/plan/nodes/dataset/DataSetUnion.scala
 ---
@@ -36,22 +39,21 @@ import scala.collection.JavaConverters._
 class DataSetUnion(
 cluster: RelOptCluster,
 traitSet: RelTraitSet,
-leftNode: RelNode,
-rightNode: RelNode,
-rowRelDataType: RelDataType)
-  extends BiRel(cluster, traitSet, leftNode, rightNode)
+inputs: JList[RelNode],
+rowRelDataType: RelDataType,
+all: Boolean)
+  extends Union(cluster, traitSet, inputs, all)
--- End diff --

Change to `Union(cluster, traitSet, inputs, true)`


---

[GitHub] flink pull request #6267: [FLINK-5750] Incorrect parse of brackets inside VA...

2018-07-05 Thread fhueske 
Github user fhueske commented on a diff in the pull request:

https://github.com/apache/flink/pull/6267#discussion_r200480534
  
--- Diff: 
flink-libraries/flink-table/src/test/java/org/apache/flink/table/runtime/batch/sql/JavaSqlITCase.java
 ---
@@ -73,6 +73,30 @@ public void testValues() throws Exception {
compareResultAsText(results, expected);
}
 
+   @Test
+   public void testValuesWithCast() throws Exception {
--- End diff --

Can you move this test to 
`org.apache.flink.table.runtime.batch.sql.SetOperatorsITCase` and also add one 
to `org.apache.flink.table.runtime.stream.sql.SetOperatorsITCase`?

In addition it would be good to have to plan tests for this query in 
`org.apache.flink.table.api.batch.sql.SetOperatorsTest` and 
`org.apache.flink.table.api.stream.sql.SetOperatorsTest`.


---

[GitHub] flink pull request #6267: [FLINK-5750] Incorrect parse of brackets inside VA...

2018-07-05 Thread fhueske 
Github user fhueske commented on a diff in the pull request:

https://github.com/apache/flink/pull/6267#discussion_r200478263
  
--- Diff: 
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/plan/nodes/dataset/DataSetUnion.scala
 ---
@@ -36,22 +39,21 @@ import scala.collection.JavaConverters._
 class DataSetUnion(
 cluster: RelOptCluster,
 traitSet: RelTraitSet,
-leftNode: RelNode,
-rightNode: RelNode,
-rowRelDataType: RelDataType)
-  extends BiRel(cluster, traitSet, leftNode, rightNode)
+inputs: JList[RelNode],
+rowRelDataType: RelDataType,
+all: Boolean)
--- End diff --

we don't need the `all` parameter because `DataStreamUnion` only supports 
`UNION ALL` semantics.


---

[GitHub] flink pull request #6267: [FLINK-5750] Incorrect parse of brackets inside VA...

2018-07-05 Thread AlexanderKoltsov 
GitHub user AlexanderKoltsov opened a pull request:

https://github.com/apache/flink/pull/6267

[FLINK-5750] Incorrect parse of brackets inside VALUES subquery

## What is the purpose of the change

*This pull request adds supporting multiple inputs in DataSetUnionRule*


## Brief change log

  - *DataSetUnionRule should consider all inputs instead of only the 1st 
and 2nd*


## Verifying this change

*This change added the following test:*
*- Added unit test testValuesWithCast that validates VALUES operator with 
values which have to to be casted. This query will be transform to UNION of 
VALUES in plan optimizer since values arguments are not literal value*

## Does this pull request potentially affect one of the following parts:

  - Dependencies (does it add or upgrade a dependency): (yes / **no**)
  - The public API, i.e., is any changed class annotated with 
`@Public(Evolving)`: (yes / **no**)
  - The serializers: (yes / **no** / don't know)
  - The runtime per-record code paths (performance sensitive): (yes / 
**no** / don't know)
  - Anything that affects deployment or recovery: JobManager (and its 
components), Checkpointing, Yarn/Mesos, ZooKeeper: (yes / **no** / don't know)
  - The S3 file system connector: (yes / **no** / don't know)

## Documentation

  - Does this pull request introduce a new feature? (yes / **no**)
  - If yes, how is the feature documented? (not applicable / docs / 
JavaDocs / not documented)

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/AlexanderKoltsov/flink master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/flink/pull/6267.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #6267


commit 6e0448b4039c577b017dd3bf2b09e68e0b53969f
Author: Alexander Koltsov 
Date:   2018-07-05T09:39:40Z

[FLINK-5750] Incorrect parse of brackets inside VALUES subquery

DataSetUnionRule should consider all inputs instead of only the 1st and 2nd.




---