You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2021/01/23 14:47:41 UTC
[GitHub] [flink] YuvalItzchakov opened a new pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
YuvalItzchakov opened a new pull request #14735:
URL: https://github.com/apache/flink/pull/14735
Fix an erroneous NPE created when using a table from a DataStream that did not assign timestamps and watermarks
## Brief change log
- Fix direct access to `ctx.timestamp()`, validate that it has a timestamp before
## Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
## Does this pull request potentially affect one of the following parts:
(Maybe, not sure) - The runtime per-record code paths (performance sensitive): (yes / no / don't know)
## Documentation
- Does this pull request introduce a new feature? No
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #14735:
URL: https://github.com/apache/flink/pull/14735#issuecomment-766092304
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.
## Automated Checks
Last check on commit 235d61be4bb5ed158c109a11de845f73c077c0fe (Sat Jan 23 15:00:49 UTC 2021)
**Warnings:**
* No documentation files were touched! Remember to keep the Flink docs up to date!
<sub>Mention the bot in a comment to re-run the automated checks.</sub>
## Review Progress
* ❓ 1. The [description] looks good.
* ❓ 2. There is [consensus] that the contribution should go into to Flink.
* ❓ 3. Needs [attention] from.
* ❓ 4. The change fits into the overall [architecture].
* ❓ 5. Overall code [quality] is good.
Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
- `@flinkbot approve all` to approve all aspects
- `@flinkbot approve-until architecture` to approve everything until `architecture`
- `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
- `@flinkbot disapprove architecture` to remove an approval you gave earlier
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14735:
URL: https://github.com/apache/flink/pull/14735#issuecomment-766094970
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12411",
"triggerID" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"triggerType" : "PUSH"
}, {
"hash" : "95137f07f2172150b444e9d3e4ce0b5cece22e2d",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12412",
"triggerID" : "95137f07f2172150b444e9d3e4ce0b5cece22e2d",
"triggerType" : "PUSH"
}, {
"hash" : "597d0277ea0046e818971d5e846e76807342b452",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12420",
"triggerID" : "597d0277ea0046e818971d5e846e76807342b452",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 95137f07f2172150b444e9d3e4ce0b5cece22e2d Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12412)
* 597d0277ea0046e818971d5e846e76807342b452 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12420)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14735:
URL: https://github.com/apache/flink/pull/14735#issuecomment-766094970
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12411",
"triggerID" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"triggerType" : "PUSH"
}, {
"hash" : "95137f07f2172150b444e9d3e4ce0b5cece22e2d",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12412",
"triggerID" : "95137f07f2172150b444e9d3e4ce0b5cece22e2d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 95137f07f2172150b444e9d3e4ce0b5cece22e2d Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12412)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14735:
URL: https://github.com/apache/flink/pull/14735#issuecomment-766094970
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12411",
"triggerID" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"triggerType" : "PUSH"
}, {
"hash" : "95137f07f2172150b444e9d3e4ce0b5cece22e2d",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12412",
"triggerID" : "95137f07f2172150b444e9d3e4ce0b5cece22e2d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 235d61be4bb5ed158c109a11de845f73c077c0fe Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12411)
* 95137f07f2172150b444e9d3e4ce0b5cece22e2d Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12412)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] YuvalItzchakov commented on pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
YuvalItzchakov commented on pull request #14735:
URL: https://github.com/apache/flink/pull/14735#issuecomment-766667105
@wuchong Reintroduced the local variable.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14735:
URL: https://github.com/apache/flink/pull/14735#issuecomment-766094970
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14735:
URL: https://github.com/apache/flink/pull/14735#issuecomment-766094970
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12411",
"triggerID" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 235d61be4bb5ed158c109a11de845f73c077c0fe Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12411)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14735:
URL: https://github.com/apache/flink/pull/14735#issuecomment-766092304
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.
## Automated Checks
Last check on commit 8d1c0382f5985cf7bdb2e9e3be979b805215a74e (Fri May 28 07:13:37 UTC 2021)
**Warnings:**
* No documentation files were touched! Remember to keep the Flink docs up to date!
<sub>Mention the bot in a comment to re-run the automated checks.</sub>
## Review Progress
* ❓ 1. The [description] looks good.
* ❓ 2. There is [consensus] that the contribution should go into to Flink.
* ❓ 3. Needs [attention] from.
* ❓ 4. The change fits into the overall [architecture].
* ❓ 5. Overall code [quality] is good.
Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
- `@flinkbot approve all` to approve all aspects
- `@flinkbot approve-until architecture` to approve everything until `architecture`
- `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
- `@flinkbot disapprove architecture` to remove an approval you gave earlier
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14735:
URL: https://github.com/apache/flink/pull/14735#issuecomment-766094970
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12411",
"triggerID" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"triggerType" : "PUSH"
}, {
"hash" : "95137f07f2172150b444e9d3e4ce0b5cece22e2d",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12412",
"triggerID" : "95137f07f2172150b444e9d3e4ce0b5cece22e2d",
"triggerType" : "PUSH"
}, {
"hash" : "597d0277ea0046e818971d5e846e76807342b452",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "597d0277ea0046e818971d5e846e76807342b452",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 95137f07f2172150b444e9d3e4ce0b5cece22e2d Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12412)
* 597d0277ea0046e818971d5e846e76807342b452 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14735:
URL: https://github.com/apache/flink/pull/14735#issuecomment-766094970
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12411",
"triggerID" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"triggerType" : "PUSH"
}, {
"hash" : "95137f07f2172150b444e9d3e4ce0b5cece22e2d",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12412",
"triggerID" : "95137f07f2172150b444e9d3e4ce0b5cece22e2d",
"triggerType" : "PUSH"
}, {
"hash" : "597d0277ea0046e818971d5e846e76807342b452",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12420",
"triggerID" : "597d0277ea0046e818971d5e846e76807342b452",
"triggerType" : "PUSH"
}, {
"hash" : "8d1c0382f5985cf7bdb2e9e3be979b805215a74e",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12448",
"triggerID" : "8d1c0382f5985cf7bdb2e9e3be979b805215a74e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8d1c0382f5985cf7bdb2e9e3be979b805215a74e Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12448)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] YuvalItzchakov commented on pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
YuvalItzchakov commented on pull request #14735:
URL: https://github.com/apache/flink/pull/14735#issuecomment-766667105
@wuchong Reintroduced the local variable.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] YuvalItzchakov commented on pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
YuvalItzchakov commented on pull request #14735:
URL: https://github.com/apache/flink/pull/14735#issuecomment-766090546
@wuchong Please re-check.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] wuchong commented on a change in pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
wuchong commented on a change in pull request #14735:
URL: https://github.com/apache/flink/pull/14735#discussion_r563227615
##########
File path: flink-table/flink-table-planner-blink/src/test/java/org/apache/flink/table/planner/runtime/stream/table/TimeAttributesITCase.scala
##########
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.planner.runtime.stream.table
+
+import org.apache.flink.api.common.eventtime.{SerializableTimestampAssigner, WatermarkStrategy}
+import org.apache.flink.api.scala.createTypeInformation
+import org.apache.flink.runtime.client.JobExecutionException
+import org.apache.flink.table.api._
+import org.apache.flink.table.planner.runtime.utils.StreamingWithStateTestBase.StateBackendMode
+import org.apache.flink.table.planner.runtime.utils.{StreamingWithStateTestBase, TestingAppendSink}
+import org.apache.flink.types.Row
+import org.junit.Assert.{assertEquals, fail}
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
+
+import java.time.{Duration, Instant, LocalDateTime, ZoneOffset}
+
+@RunWith(classOf[Parameterized])
+class TimeAttributesITCase(mode: StateBackendMode) extends StreamingWithStateTestBase(mode) {
+
+ @Test
+ def testMissingTimeAttributeThrowsCorrectException(): Unit = {
+ val data = List(1L -> "hello", 2L -> "world")
+ val stream = env.fromCollection[(Long, String)](data)
+
+ tEnv.createTemporaryView("test", stream, $"event_time".rowtime(), $"data")
+ val result = tEnv.sqlQuery("SELECT * FROM test")
+
+ val sink = new TestingAppendSink()
+ tEnv.toAppendStream[Row](result).addSink(sink)
+ try {
+ env.execute()
+ } catch {
+ case je: JobExecutionException =>
+ val innerCause = je.getCause.getCause
+ assert(innerCause.isInstanceOf[RuntimeException])
+ assertEquals(
+ "Rowtime timestamp is not defined. Please make sure that a " +
+ "proper TimestampAssigner is defined and the stream environment uses the EventTime " +
+ "time characteristic.",
+ innerCause.getMessage)
+ case e: Exception => fail(s"Expected JobExecutionException, received $e")
Review comment:
Can be simplify to
```scala
case t: Throwable =>
assertThat(
t,
containsMessage("Rowtime timestamp is not defined. Please make sure that a " +
"proper TimestampAssigner is defined and the stream environment uses the EventTime " +
"time characteristic."))
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] wuchong edited a comment on pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
wuchong edited a comment on pull request #14735:
URL: https://github.com/apache/flink/pull/14735#issuecomment-766353650
Oops, `MatchRecognizeITCase.testWindowedGroupingAppliedToMatchRecognize` is still failed, because `org.apache.flink.cep.time.TimeContext#timestamp()` returns primitive type `long`, and `long` can't be compared with `null`.
I think we either we generate a special code for `MATCH_ROWTIME` in `MatchCodeGenerator#visitCall`, either faillback your previous approach by introducing a temporary `Long` variable.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14735:
URL: https://github.com/apache/flink/pull/14735#issuecomment-766094970
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12411",
"triggerID" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"triggerType" : "PUSH"
}, {
"hash" : "95137f07f2172150b444e9d3e4ce0b5cece22e2d",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12412",
"triggerID" : "95137f07f2172150b444e9d3e4ce0b5cece22e2d",
"triggerType" : "PUSH"
}, {
"hash" : "597d0277ea0046e818971d5e846e76807342b452",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12420",
"triggerID" : "597d0277ea0046e818971d5e846e76807342b452",
"triggerType" : "PUSH"
}, {
"hash" : "8d1c0382f5985cf7bdb2e9e3be979b805215a74e",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "8d1c0382f5985cf7bdb2e9e3be979b805215a74e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 597d0277ea0046e818971d5e846e76807342b452 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12420)
* 8d1c0382f5985cf7bdb2e9e3be979b805215a74e UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] YuvalItzchakov commented on pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
YuvalItzchakov commented on pull request #14735:
URL: https://github.com/apache/flink/pull/14735#issuecomment-766310391
@wuchong Thank you for the comments. Made the modifications.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] wuchong merged pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
wuchong merged pull request #14735:
URL: https://github.com/apache/flink/pull/14735
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] wuchong edited a comment on pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
wuchong edited a comment on pull request #14735:
URL: https://github.com/apache/flink/pull/14735#issuecomment-766353650
Oops, `MatchRecognizeITCase.testWindowedGroupingAppliedToMatchRecognize` is still failed, because `org.apache.flink.cep.time.TimeContext#timestamp()` returns primitive type `long`, and `long` can't be compared with `null`.
I think we can either generate a special code for `MATCH_ROWTIME` in `MatchCodeGenerator#visitCall`, or fallback to your previous approach by introducing a temporary `Long` variable.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14735:
URL: https://github.com/apache/flink/pull/14735#issuecomment-766094970
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12411",
"triggerID" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"triggerType" : "PUSH"
}, {
"hash" : "95137f07f2172150b444e9d3e4ce0b5cece22e2d",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12412",
"triggerID" : "95137f07f2172150b444e9d3e4ce0b5cece22e2d",
"triggerType" : "PUSH"
}, {
"hash" : "597d0277ea0046e818971d5e846e76807342b452",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12420",
"triggerID" : "597d0277ea0046e818971d5e846e76807342b452",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 597d0277ea0046e818971d5e846e76807342b452 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12420)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] wuchong merged pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
wuchong merged pull request #14735:
URL: https://github.com/apache/flink/pull/14735
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14735:
URL: https://github.com/apache/flink/pull/14735#issuecomment-766094970
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12411",
"triggerID" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"triggerType" : "PUSH"
}, {
"hash" : "95137f07f2172150b444e9d3e4ce0b5cece22e2d",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12412",
"triggerID" : "95137f07f2172150b444e9d3e4ce0b5cece22e2d",
"triggerType" : "PUSH"
}, {
"hash" : "597d0277ea0046e818971d5e846e76807342b452",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12420",
"triggerID" : "597d0277ea0046e818971d5e846e76807342b452",
"triggerType" : "PUSH"
}, {
"hash" : "8d1c0382f5985cf7bdb2e9e3be979b805215a74e",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12448",
"triggerID" : "8d1c0382f5985cf7bdb2e9e3be979b805215a74e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 597d0277ea0046e818971d5e846e76807342b452 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12420)
* 8d1c0382f5985cf7bdb2e9e3be979b805215a74e Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12448)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14735:
URL: https://github.com/apache/flink/pull/14735#issuecomment-766094970
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12411",
"triggerID" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"triggerType" : "PUSH"
}, {
"hash" : "95137f07f2172150b444e9d3e4ce0b5cece22e2d",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "95137f07f2172150b444e9d3e4ce0b5cece22e2d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 235d61be4bb5ed158c109a11de845f73c077c0fe Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12411)
* 95137f07f2172150b444e9d3e4ce0b5cece22e2d UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] wuchong commented on a change in pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
wuchong commented on a change in pull request #14735:
URL: https://github.com/apache/flink/pull/14735#discussion_r563227674
##########
File path: flink-table/flink-table-planner-blink/src/test/java/org/apache/flink/table/planner/runtime/stream/table/TimeAttributesITCase.scala
##########
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.planner.runtime.stream.table
+
+import org.apache.flink.api.common.eventtime.{SerializableTimestampAssigner, WatermarkStrategy}
+import org.apache.flink.api.scala.createTypeInformation
+import org.apache.flink.runtime.client.JobExecutionException
+import org.apache.flink.table.api._
+import org.apache.flink.table.planner.runtime.utils.StreamingWithStateTestBase.StateBackendMode
+import org.apache.flink.table.planner.runtime.utils.{StreamingWithStateTestBase, TestingAppendSink}
+import org.apache.flink.types.Row
+import org.junit.Assert.{assertEquals, fail}
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
+
+import java.time.{Duration, Instant, LocalDateTime, ZoneOffset}
+
+@RunWith(classOf[Parameterized])
+class TimeAttributesITCase(mode: StateBackendMode) extends StreamingWithStateTestBase(mode) {
+
+ @Test
+ def testMissingTimeAttributeThrowsCorrectException(): Unit = {
+ val data = List(1L -> "hello", 2L -> "world")
+ val stream = env.fromCollection[(Long, String)](data)
+
+ tEnv.createTemporaryView("test", stream, $"event_time".rowtime(), $"data")
+ val result = tEnv.sqlQuery("SELECT * FROM test")
+
+ val sink = new TestingAppendSink()
+ tEnv.toAppendStream[Row](result).addSink(sink)
+ try {
+ env.execute()
Review comment:
Should add `fail("should fail")` to make sure there is a runtime exception when running the query.
##########
File path: flink-table/flink-table-planner-blink/src/test/java/org/apache/flink/table/planner/runtime/stream/table/TimeAttributesITCase.scala
##########
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.planner.runtime.stream.table
+
+import org.apache.flink.api.common.eventtime.{SerializableTimestampAssigner, WatermarkStrategy}
+import org.apache.flink.api.scala.createTypeInformation
+import org.apache.flink.runtime.client.JobExecutionException
+import org.apache.flink.table.api._
+import org.apache.flink.table.planner.runtime.utils.StreamingWithStateTestBase.StateBackendMode
+import org.apache.flink.table.planner.runtime.utils.{StreamingWithStateTestBase, TestingAppendSink}
+import org.apache.flink.types.Row
+import org.junit.Assert.{assertEquals, fail}
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
+
+import java.time.{Duration, Instant, LocalDateTime, ZoneOffset}
+
+@RunWith(classOf[Parameterized])
+class TimeAttributesITCase(mode: StateBackendMode) extends StreamingWithStateTestBase(mode) {
+
+ @Test
+ def testMissingTimeAttributeThrowsCorrectException(): Unit = {
+ val data = List(1L -> "hello", 2L -> "world")
+ val stream = env.fromCollection[(Long, String)](data)
+
+ tEnv.createTemporaryView("test", stream, $"event_time".rowtime(), $"data")
+ val result = tEnv.sqlQuery("SELECT * FROM test")
+
+ val sink = new TestingAppendSink()
+ tEnv.toAppendStream[Row](result).addSink(sink)
+ try {
+ env.execute()
+ } catch {
+ case je: JobExecutionException =>
+ val innerCause = je.getCause.getCause
+ assert(innerCause.isInstanceOf[RuntimeException])
+ assertEquals(
+ "Rowtime timestamp is not defined. Please make sure that a " +
+ "proper TimestampAssigner is defined and the stream environment uses the EventTime " +
+ "time characteristic.",
+ innerCause.getMessage)
+ case e: Exception => fail(s"Expected JobExecutionException, received $e")
Review comment:
Can be simplify to
```scala
assertThat(
t,
containsMessage("Rowtime timestamp is not defined. Please make sure that a " +
"proper TimestampAssigner is defined and the stream environment uses the EventTime " +
"time characteristic."))
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #14735:
URL: https://github.com/apache/flink/pull/14735#issuecomment-766094970
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "235d61be4bb5ed158c109a11de845f73c077c0fe",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 235d61be4bb5ed158c109a11de845f73c077c0fe UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] wuchong commented on pull request #14735: [FLINK-20961][table-planner-blink] Fix NPE when no assigned timestamp defined
Posted by GitBox <gi...@apache.org>.
wuchong commented on pull request #14735:
URL: https://github.com/apache/flink/pull/14735#issuecomment-766353650
Oops, `MatchRecognizeITCase.testWindowedGroupingAppliedToMatchRecognize` is still failed, because `org.apache.flink.cep.time.TimeContext#timestamp` returns primitive type `long`, and `long` can't be compared with `null`.
I think we either we generate a special code for `MATCH_ROWTIME` in `MatchCodeGenerator#visitCall`, either faillback your previous approach by introducing a temporary `Long` variable.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org