You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by snuyanzin <gi...@git.apache.org> on 2018/05/07 09:51:52 UTC

[GitHub] flink pull request #5961: [Flink-8255][DataSet API, DataStream API] key expr...

GitHub user snuyanzin opened a pull request:

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

    [Flink-8255][DataSet API, DataStream API] key expressions on named row types do not work

    *Thank you very much for contributing to Apache Flink - we are happy that you want to help us improve Flink. To help the community review your contribution in the best possible way, please go through the checklist below, which will get the contribution into a shape in which it can be best reviewed.*
    
    *Please understand that we do not do this to make contributions to Flink a hassle. In order to uphold a high standard of quality for code contributions, while at the same time managing a large number of contributions, we need contributors to prepare the contributions well, and give reviewers enough contextual information for the review. Please also understand that contributions that do not follow this guide will take longer to review and thus typically be picked up with lower priority by the community.*
    
    ## Contribution Checklist
    
      - Make sure that the pull request corresponds to a [JIRA issue](https://issues.apache.org/jira/projects/FLINK/issues). Exceptions are made for typos in JavaDoc or documentation files, which need no JIRA issue.
      
      - Name the pull request in the form "[FLINK-XXXX] [component] Title of the pull request", where *FLINK-XXXX* should be replaced by the actual issue number. Skip *component* if you are unsure about which is the best component.
      Typo fixes that have no associated JIRA issue should be named following this pattern: `[hotfix] [docs] Fix typo in event time introduction` or `[hotfix] [javadocs] Expand JavaDoc for PuncuatedWatermarkGenerator`.
    
      - Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
      
      - Make sure that the change passes the automated tests, i.e., `mvn clean verify` passes. You can set up Travis CI to do that following [this guide](http://flink.apache.org/contribute-code.html#best-practices).
    
      - Each pull request should address only one issue, not mix up code from multiple issues.
      
      - Each commit in the pull request has a meaningful commit message (including the JIRA id)
    
      - Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below.
    
    
    **(The sections below can be removed for hotfixes of typos)**
    
    ## What is the purpose of the change
    Fix issues related to ClassCastExceptions from Flink-8255 + add more tests
    
    ## Brief change log
    - Usage of casting to TupleTypeInfoBase rather than to TupleTypeInfo as RowTypeInfo is a child of TupleTypeInfoBase but in a different branch in compare with TupleTypeInfo
    - Add more tests which will fail with ClassCastException without changes from the previous item
    
    ## Verifying this change
    
    *(Please pick either of the following options)*
    This change added tests and can be verified as follows:
    - Added tests that validates that ClassCastException related to Flink-8255 are not happen
    
    ## Does this pull request potentially affect one of the following parts:
    
      - Dependencies (does it add or upgrade a dependency): (no)
      - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes)
      - The serializers: (don't know)
      - The runtime per-record code paths (performance sensitive): (don't know)
      - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
      - The S3 file system connector: (no)
    
    ## Documentation
    
      - Does this pull request introduce a new feature? (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/snuyanzin/flink FLINK-8255_Key_expressions_on_named_row_types_do_not_work

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

    https://github.com/apache/flink/pull/5961.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 #5961
    
----
commit 50eb4ead75ddc22e0eee2bf5a2b9d12c37dcaeb4
Author: snuyanzin <sn...@...>
Date:   2018-04-29T18:37:05Z

    FLINK-8255 Key expressions on named row types do not work
    Test case from description + 2 more
    Resolution of class cast by using of TupleTypeInfoBase rather than TupleTypeInfo

commit 96d569025e21172059bb5b34cf93f3a60b5f0a0e
Author: snuyanzin <sn...@...>
Date:   2018-04-29T18:47:52Z

    FLINK-8255 Key expressions on named row types do not work
    Test case from description + 2 more
    Resolution of class cast by using of TupleTypeInfoBase rather than TupleTypeInfo

commit 889f226b2268e5e23ed3de672a60a05702c111aa
Author: snuyanzin <sn...@...>
Date:   2018-04-29T20:30:04Z

    FLINK-8255 Key expressions on named row types do not work
    Test case from description + 2 more
    Resolution of class cast by using of TupleTypeInfoBase rather than TupleTypeInfo

commit 93dd630dd5a833a0e343a5ff53d0c5e5f37934cd
Author: snuyanzin <sn...@...>
Date:   2018-05-07T05:13:14Z

    FLINK-8255 Key expressions on named row types do not work
    Test case from description + 2 more
    Resolution of class cast by using of TupleTypeInfoBase rather than TupleTypeInfo

commit dc20cca498c2758d1cc0fa87bc51461a564e41f7
Author: snuyanzin <sn...@...>
Date:   2018-05-07T05:40:10Z

    FLINK-8255 Key expressions on named row types do not work
    Test case from description + 2 more
    Resolution of class cast by using of TupleTypeInfoBase rather than TupleTypeInfo

commit b1f33282c30cbc6ee7b9d6e3b595fc03955346b0
Author: snuyanzin <sn...@...>
Date:   2018-05-07T05:45:04Z

    FLINK-8255 Key expressions on named row types do not work
    Test case from description + 2 more
    Resolution of class cast by using of TupleTypeInfoBase rather than TupleTypeInfo

commit 295d0fc719010783da591781e9c4ab60e3e216e4
Author: snuyanzin <sn...@...>
Date:   2018-05-07T08:15:12Z

    FLINK-8255 Key expressions on named row types do not work
    Test case from description + 2 more
    Resolution of class cast by using of TupleTypeInfoBase rather than TupleTypeInfo

----


---

[GitHub] flink pull request #5961: [FLINK-8255][DataSet API, DataStream API] key expr...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5961#discussion_r186527205
  
    --- Diff: flink-streaming-java/src/main/java/org/apache/flink/streaming/util/typeutils/FieldAccessor.java ---
    @@ -157,15 +156,15 @@ public T set(T record, F fieldValue) {
     
     		SimpleTupleFieldAccessor(int pos, TypeInformation<T> typeInfo) {
    --- End diff --
    
    accessing fields in a `Row` will fail because `Row` does not extend `Tuple`. For a proper fix, we would need a `RowFieldAccessor` and use that one when we deal with a `DataStream<Row>`. We would then need to add the `RowFieldAccessor` to the `FieldAccessorFactory`.


---

[GitHub] flink pull request #5961: [FLINK-8255][DataSet API, DataStream API] key expr...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5961#discussion_r186527277
  
    --- Diff: flink-streaming-java/src/main/java/org/apache/flink/streaming/util/typeutils/FieldAccessor.java ---
    @@ -197,7 +196,7 @@ public T set(T record, F fieldValue) {
     			checkNotNull(typeInfo, "typeInfo must not be null.");
     			checkNotNull(innerAccessor, "innerAccessor must not be null.");
     
    -			int arity = ((TupleTypeInfo) typeInfo).getArity();
    +			int arity = typeInfo.getArity();
    --- End diff --
    
    Same as for `SimpleTupleFieldAccessor`.


---

[GitHub] flink issue #5961: [FLINK-8255][DataSet API, DataStream API] key expressions...

Posted by snuyanzin <gi...@git.apache.org>.
Github user snuyanzin commented on the issue:

    https://github.com/apache/flink/pull/5961
  
    Hello @fhueske 
    Thank you for your review
    
    As you proposed I tried to use org.apache.flink.api.java.typeutils.RowTypeInfo#isTupleType
    ```java
    public boolean isTupleType() {
    		return false;
    	}
    ```
    however after that these tests started to fail
    org.apache.flink.table.api.batch.ExplainTest#testJoinWithoutExtended
    org.apache.flink.table.api.batch.ExplainTest#testJoinWithExtended
    
    like 
    ```
    testJoinWithoutExtended(org.apache.flink.table.api.batch.ExplainTest)  Time elapsed: 0.037 sec  <<< ERROR!
    
    org.apache.flink.api.common.InvalidProgramException: Specifying keys via field positions is only valid for tuple data types. Type: Row(a: Integer, b: String)
    
    	at org.apache.flink.api.common.operators.Keys$ExpressionKeys.<init>(Keys.java:232)
    
    	at org.apache.flink.api.common.operators.Keys$ExpressionKeys.<init>(Keys.java:223)
    
    	at org.apache.flink.api.java.operators.JoinOperator$JoinOperatorSets.where(JoinOperator.java:901)
    
    	at org.apache.flink.table.plan.nodes.dataset.DataSetJoin.addInnerJoin(DataSetJoin.scala:243)
    
    	at org.apache.flink.table.plan.nodes.dataset.DataSetJoin.translateToPlan(DataSetJoin.scala:170)
    
    	at org.apache.flink.table.plan.nodes.dataset.DataSetCalc.translateToPlan(DataSetCalc.scala:91)
    
    	at org.apache.flink.table.api.BatchTableEnvironment.translate(BatchTableEnvironment.scala:422)
    
    	at org.apache.flink.table.api.BatchTableEnvironment.explain(BatchTableEnvironment.scala:249)
    
    	at org.apache.flink.table.api.BatchTableEnvironment.explain(BatchTableEnvironment.scala:275)
    
    	at org.apache.flink.table.api.batch.ExplainTest.testJoinWithoutExtended(ExplainTest.scala:72)
    ```
    
    that is why I decided to use instanceof TupleTypeInfo check as anyway next line there is a cast to this type
    
    could you please have a look at it and tell if it is acceptable or not?


---

[GitHub] flink pull request #5961: [FLINK-8255][DataSet API, DataStream API] key expr...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5961#discussion_r186484649
  
    --- Diff: flink-java/src/test/java/org/apache/flink/api/java/operator/MaxByOperatorTest.java ---
    @@ -230,4 +235,43 @@ public String toString() {
     		}
     	}
     
    +	/**
    +	 * Validates that no ClassCastException happens
    +	 * should not fail e.g. like in FLINK-8255.
    +	 */
    +	@Test
    +	public void testMaxMinByRowTypeInfoKeyFieldsDataset() {
    +
    +		final ExecutionEnvironment env = ExecutionEnvironment
    +			.getExecutionEnvironment();
    +		TypeInformation[] types = new TypeInformation[] {Types.INT, Types.INT};
    +
    +		String[] fieldNames = new String[]{"id", "value"};
    +		RowTypeInfo rowTypeInfo = new RowTypeInfo(types, fieldNames);
    +		DataSet tupleDs = env
    +			.fromCollection(Collections.singleton(new Row(2)), rowTypeInfo);
    +
    +		tupleDs.maxBy(0);
    +		tupleDs.minBy(0);
    +	}
    +
    +    /**
    +     * Validates that no ClassCastException happens
    +	 * should not fail e.g. like in FLINK-8255.
    +	 */
    +	@Test
    +	public void testMaxMinByRowTypeInfoKeyFieldsForUnsortedGrouping() {
    +		final ExecutionEnvironment env = ExecutionEnvironment.getExecutionEnvironment();
    +
    +		TypeInformation[] types = new TypeInformation[]{Types.INT, Types.INT};
    +
    +		String[] fieldNames = new String[]{"id", "value"};
    +		RowTypeInfo rowTypeInfo = new RowTypeInfo(types, fieldNames);
    +
    +		UnsortedGrouping groupDs = env.fromCollection(Collections.singleton(new Row(2)), rowTypeInfo).groupBy(0);
    +
    +		groupDs.maxBy(1);
    +		groupDs.minBy(1);
    --- End diff --
    
    The tests pass because the program is not executed. 
    You would have to call `env.collect()` to run the program and compare the returned result against the expected result. As I pointed out before, this will fail, because the operator will cast the `Row` objects to `Tuple`.


---

[GitHub] flink pull request #5961: [FLINK-8255][DataSet API, DataStream API] key expr...

Posted by snuyanzin <gi...@git.apache.org>.
GitHub user snuyanzin reopened a pull request:

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

    [FLINK-8255][DataSet API, DataStream API] key expressions on named row types do not work

    *Thank you very much for contributing to Apache Flink - we are happy that you want to help us improve Flink. To help the community review your contribution in the best possible way, please go through the checklist below, which will get the contribution into a shape in which it can be best reviewed.*
    
    *Please understand that we do not do this to make contributions to Flink a hassle. In order to uphold a high standard of quality for code contributions, while at the same time managing a large number of contributions, we need contributors to prepare the contributions well, and give reviewers enough contextual information for the review. Please also understand that contributions that do not follow this guide will take longer to review and thus typically be picked up with lower priority by the community.*
    
    ## Contribution Checklist
    
      - Make sure that the pull request corresponds to a [JIRA issue](https://issues.apache.org/jira/projects/FLINK/issues). Exceptions are made for typos in JavaDoc or documentation files, which need no JIRA issue.
      
      - Name the pull request in the form "[FLINK-XXXX] [component] Title of the pull request", where *FLINK-XXXX* should be replaced by the actual issue number. Skip *component* if you are unsure about which is the best component.
      Typo fixes that have no associated JIRA issue should be named following this pattern: `[hotfix] [docs] Fix typo in event time introduction` or `[hotfix] [javadocs] Expand JavaDoc for PuncuatedWatermarkGenerator`.
    
      - Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
      
      - Make sure that the change passes the automated tests, i.e., `mvn clean verify` passes. You can set up Travis CI to do that following [this guide](http://flink.apache.org/contribute-code.html#best-practices).
    
      - Each pull request should address only one issue, not mix up code from multiple issues.
      
      - Each commit in the pull request has a meaningful commit message (including the JIRA id)
    
      - Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below.
    
    
    **(The sections below can be removed for hotfixes of typos)**
    
    ## What is the purpose of the change
    Fix issues related to ClassCastExceptions from Flink-8255 + add more tests
    
    ## Brief change log
    - Usage of casting to TupleTypeInfoBase rather than to TupleTypeInfo as RowTypeInfo is a child of TupleTypeInfoBase but in a different branch in compare with TupleTypeInfo
    - Add more tests which will fail with ClassCastException without changes from the previous item
    
    ## Verifying this change
    
    *(Please pick either of the following options)*
    This change added tests and can be verified as follows:
    - Added tests that validates that ClassCastException related to Flink-8255 are not happen
    
    ## Does this pull request potentially affect one of the following parts:
    
      - Dependencies (does it add or upgrade a dependency): (no)
      - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes)
      - The serializers: (don't know)
      - The runtime per-record code paths (performance sensitive): (don't know)
      - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
      - The S3 file system connector: (no)
    
    ## Documentation
    
      - Does this pull request introduce a new feature? (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/snuyanzin/flink FLINK-8255_Key_expressions_on_named_row_types_do_not_work

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

    https://github.com/apache/flink/pull/5961.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 #5961
    
----
commit 50eb4ead75ddc22e0eee2bf5a2b9d12c37dcaeb4
Author: snuyanzin <sn...@...>
Date:   2018-04-29T18:37:05Z

    FLINK-8255 Key expressions on named row types do not work
    Test case from description + 2 more
    Resolution of class cast by using of TupleTypeInfoBase rather than TupleTypeInfo

commit 96d569025e21172059bb5b34cf93f3a60b5f0a0e
Author: snuyanzin <sn...@...>
Date:   2018-04-29T18:47:52Z

    FLINK-8255 Key expressions on named row types do not work
    Test case from description + 2 more
    Resolution of class cast by using of TupleTypeInfoBase rather than TupleTypeInfo

commit 889f226b2268e5e23ed3de672a60a05702c111aa
Author: snuyanzin <sn...@...>
Date:   2018-04-29T20:30:04Z

    FLINK-8255 Key expressions on named row types do not work
    Test case from description + 2 more
    Resolution of class cast by using of TupleTypeInfoBase rather than TupleTypeInfo

commit 93dd630dd5a833a0e343a5ff53d0c5e5f37934cd
Author: snuyanzin <sn...@...>
Date:   2018-05-07T05:13:14Z

    FLINK-8255 Key expressions on named row types do not work
    Test case from description + 2 more
    Resolution of class cast by using of TupleTypeInfoBase rather than TupleTypeInfo

commit dc20cca498c2758d1cc0fa87bc51461a564e41f7
Author: snuyanzin <sn...@...>
Date:   2018-05-07T05:40:10Z

    FLINK-8255 Key expressions on named row types do not work
    Test case from description + 2 more
    Resolution of class cast by using of TupleTypeInfoBase rather than TupleTypeInfo

commit b1f33282c30cbc6ee7b9d6e3b595fc03955346b0
Author: snuyanzin <sn...@...>
Date:   2018-05-07T05:45:04Z

    FLINK-8255 Key expressions on named row types do not work
    Test case from description + 2 more
    Resolution of class cast by using of TupleTypeInfoBase rather than TupleTypeInfo

commit 295d0fc719010783da591781e9c4ab60e3e216e4
Author: snuyanzin <sn...@...>
Date:   2018-05-07T08:15:12Z

    FLINK-8255 Key expressions on named row types do not work
    Test case from description + 2 more
    Resolution of class cast by using of TupleTypeInfoBase rather than TupleTypeInfo

commit adbd53d06cb335e62a0022835f352b62a809b128
Author: snuyanzin <sn...@...>
Date:   2018-05-10T11:18:27Z

    Merge remote-tracking branch 'upstream/master' into HEAD

commit 8a87fb895d516c2f651c98cdc020d895d6203f25
Author: snuyanzin <sn...@...>
Date:   2018-05-10T11:24:36Z

    FLINK-8255_Key_expressions_on_named_row_types_do_not_work
    corrections based on comment https://issues.apache.org/jira/browse/FLINK-8255?focusedCommentId=16466381&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16466381

commit fb519774fbeac7774fe9a4ef9c44d16b98443242
Author: snuyanzin <sn...@...>
Date:   2018-05-10T13:26:50Z

    FLINK-8255_Key_expressions_on_named_row_types_do_not_work
    Tests corrected

commit 9af3290fa93802feeef8503d8b5ff39b161f2072
Author: snuyanzin <sn...@...>
Date:   2018-05-10T13:37:45Z

    Merge branch 'FLINK-8255_Key_expressions_on_named_row_types_do_not_work' of https://github.com/snuyanzin/flink into FLINK-8255_Key_expressions_on_named_row_types_do_not_work

commit 470d91086c8f49397011dba713a5bcde77d4e60e
Author: snuyanzin <sn...@...>
Date:   2018-05-10T13:46:41Z

    FLINK-8255_Key_expressions_on_named_row_types_do_not_work
    fix imports to make them unchanged as initially

commit 82af1a42426bd8600718ac71b5fbc26fc48ead23
Author: snuyanzin <sn...@...>
Date:   2018-05-10T14:24:16Z

    [FLINK-8518][Table API and SQL] Support DOW, EPOCH, DECADE for EXTRACT
    DOW implementation

commit 3387361444d099352c6ec15055a06033206b0e02
Author: snuyanzin <sn...@...>
Date:   2018-05-10T14:26:08Z

    Revert "[FLINK-8518][Table API and SQL] Support DOW, EPOCH, DECADE for EXTRACT"
    
    This reverts commit 82af1a42426bd8600718ac71b5fbc26fc48ead23.

commit ebab3b5d88b50f1bdd18da4707688e98588cf729
Author: snuyanzin <sn...@...>
Date:   2018-05-10T14:30:28Z

    [FLINK-8518][Table API and SQL] Support DOW, EPOCH, DECADE for EXTRACT
    DOW implementation

commit 9aeea218822c552886b8497a67ef33a8906c94dc
Author: snuyanzin <sn...@...>
Date:   2018-05-10T14:31:20Z

    Revert "[FLINK-8518][Table API and SQL] Support DOW, EPOCH, DECADE for EXTRACT"
    
    This reverts commit ebab3b5d88b50f1bdd18da4707688e98588cf729.

commit 125fd792e4245759a703b168d5febf5adcfff6c3
Author: snuyanzin <sn...@...>
Date:   2018-05-10T14:35:09Z

    [FLINK-8518][Table API and SQL] Support DOW, EPOCH, DECADE for EXTRACT
    DOW implementation

commit d891cc5f04bb84861c55e238c9ffff9599efa980
Author: snuyanzin <sn...@...>
Date:   2018-05-10T14:48:19Z

    FLINK-8255_Key_expressions_on_named_row_types_do_not_work
    added expected exception type

commit 497aa2071947e7b2f513504b37c98ca0067d4460
Author: snuyanzin <sn...@...>
Date:   2018-05-10T14:50:41Z

    FLINK-8255_Key_expressions_on_named_row_types_do_not_work
    omit issue not related changes

commit 903db8cb13eb3774eb6daf3d6ed46a93a27062d6
Author: snuyanzin <sn...@...>
Date:   2018-05-14T13:31:55Z

    FLINK-8255_Key_expressions_on_named_row_types_do_not_work
    use instanceof as
    org.apache.flink.api.java.typeutils.RowTypeInfo#isTupleType leads to
    org.apache.flink.table.api.batch.ExplainTest#testJoinWithoutExtended failure

commit 9f54cf7bb3fbc60c0be4209a3f33950ba8b9794c
Author: snuyanzin <sn...@...>
Date:   2018-05-14T13:33:36Z

    Revert "FLINK-8255_Key_expressions_on_named_row_types_do_not_work"
    
    This reverts commit 497aa2071947e7b2f513504b37c98ca0067d4460.

commit d616dd47b22d10e6f5edb1fe363e1010b760544e
Author: snuyanzin <sn...@...>
Date:   2018-05-14T13:35:54Z

    Revert "FLINK-8255_Key_expressions_on_named_row_types_do_not_work"
    
    This reverts commit d891cc5f04bb84861c55e238c9ffff9599efa980.

commit ff8fde21fed6f793af2046570b3c839edb00a426
Author: snuyanzin <sn...@...>
Date:   2018-05-14T13:42:39Z

    FLINK-8255_Key_expressions_on_named_row_types_do_not_work
    omit issue not relate changes

commit a7994adc9804dca205773591e42d8f56225e5dfb
Author: snuyanzin <sn...@...>
Date:   2018-05-14T13:46:08Z

    FLINK-8255_Key_expressions_on_named_row_types_do_not_work
    added expected exception to tests

commit 07fad75781474163d546641be0d3902c758de2c8
Author: snuyanzin <sn...@...>
Date:   2018-05-14T14:11:46Z

    FLINK-8255_Key_expressions_on_named_row_types_do_not_work
    added instanceof checks for DataSet

commit 983cf4fff2cacbbf00922b142b4a5a09b6a74224
Author: snuyanzin <sn...@...>
Date:   2018-05-10T11:24:36Z

    FLINK-8255_Key_expressions_on_named_row_types_do_not_work
    corrections based on comment https://issues.apache.org/jira/browse/FLINK-8255?focusedCommentId=16466381&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16466381

commit 1065d21e762c5bfc1867bc729812f8d013dd5838
Author: snuyanzin <sn...@...>
Date:   2018-05-10T13:26:50Z

    FLINK-8255_Key_expressions_on_named_row_types_do_not_work
    Tests corrected

commit c1d754c58a9aad208952fad0d4a10a5b6bcfed66
Author: snuyanzin <sn...@...>
Date:   2018-05-10T13:46:41Z

    FLINK-8255_Key_expressions_on_named_row_types_do_not_work
    fix imports to make them unchanged as initially

commit aa4cf128bfd387b18dc618947e55ba27303e8e44
Author: snuyanzin <sn...@...>
Date:   2018-05-10T14:24:16Z

    [FLINK-8518][Table API and SQL] Support DOW, EPOCH, DECADE for EXTRACT
    DOW implementation

commit 98a695166c53d794fdc98d0521e247d0c70154b5
Author: snuyanzin <sn...@...>
Date:   2018-05-10T14:26:08Z

    Revert "[FLINK-8518][Table API and SQL] Support DOW, EPOCH, DECADE for EXTRACT"
    
    This reverts commit 82af1a42426bd8600718ac71b5fbc26fc48ead23.

----


---

[GitHub] flink issue #5961: [FLINK-8255][DataSet API, DataStream API] key expressions...

Posted by snuyanzin <gi...@git.apache.org>.
Github user snuyanzin commented on the issue:

    https://github.com/apache/flink/pull/5961
  
    commits squashed into one 


---

[GitHub] flink pull request #5961: [FLINK-8255][DataSet API, DataStream API] key expr...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5961#discussion_r186528901
  
    --- Diff: flink-streaming-java/src/test/java/org/apache/flink/streaming/util/typeutils/FieldAccessorTest.java ---
    @@ -368,4 +369,23 @@ public void testIllegalBasicType2() {
     
     		FieldAccessor<Long, Long> f = FieldAccessorFactory.getAccessor(tpeInfo, "foo", null);
     	}
    +
    +	/**
    +	 * Validates that no ClassCastException happens
    +	 * should not fail e.g. like in FLINK-8255.
    +	 */
    +	@Test
    +	public void testRowTypeInfo() {
    --- End diff --
    
    This test just validates that a `FieldAccessor` is created. At runtime it would fail with a `ClassCastException`.


---

[GitHub] flink pull request #5961: [FLINK-8255][DataSet API, DataStream API] key expr...

Posted by snuyanzin <gi...@git.apache.org>.
Github user snuyanzin closed the pull request at:

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


---

[GitHub] flink pull request #5961: [FLINK-8255][DataSet API, DataStream API] key expr...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5961#discussion_r186477977
  
    --- Diff: flink-java/src/main/java/org/apache/flink/api/java/functions/SelectByMinFunction.java ---
    @@ -41,7 +41,7 @@
     	 * is regarded in the reduce function. First index has highest priority and last index has
     	 * least priority.
     	 */
    -	public SelectByMinFunction(TupleTypeInfo<T> type, int... fields) {
    +	public SelectByMinFunction(TupleTypeInfoBase<T> type, int... fields) {
    --- End diff --
    
    The `ReduceFunction` is still typed to `T extends Tuple` such that this will still fail at runtime. The same is true for all other built-in aggregation method like `sum()` and `min()` on `DataSet` and `UnsortedGrouping`. 
    
    This cannot be resolved without major changes. I don't think we should add these features, but rather throw meaningful error messages instead of `ClassCastException`. 
    
    Can you try to override the the `isTupleType()` method in `RowTypeInfo` and return `false`? 
    This would prevent `Row` from being used in contexts that are only supported for `Tuple`.


---

[GitHub] flink issue #5961: [FLINK-8255][DataSet API, DataStream API] key expressions...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:

    https://github.com/apache/flink/pull/5961
  
    Thanks for the update @snuyanzin. I'll try to have a look at the changes in the next days.
    Best, Fabian


---

[GitHub] flink issue #5961: [FLINK-8255][DataSet API, DataStream API] key expressions...

Posted by snuyanzin <gi...@git.apache.org>.
Github user snuyanzin commented on the issue:

    https://github.com/apache/flink/pull/5961
  
    sorry, was closed by mistake
    reopened


---