You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by sunjincheng121 <gi...@git.apache.org> on 2017/06/11 13:31:48 UTC
[GitHub] flink pull request #4102: [FLINK-6886][table]Fix Timestamp field can not be ...
GitHub user sunjincheng121 opened a pull request:
https://github.com/apache/flink/pull/4102
[FLINK-6886][table]Fix Timestamp field can not be selected in event t…
In this JIRA. will fix the exception when we run the folllows SQL:
`SELECT name, max(num) as myMax, TUMBLE_START(rowtime, INTERVAL '5' SECOND) as winStart,TUMBLE_END(rowtime, INTERVAL '5' SECOND) as winEnd FROM T1 GROUP BY name, TUMBLE(rowtime, INTERVAL '5' SECOND)`
Exception Info:
```
org.apache.flink.table.api.TableException: The field types of physical and logical row types do not match.This is a bug and should not happen. Please file an issue.
at org.apache.flink.table.api.TableException$.apply(exceptions.scala:53)
at org.apache.flink.table.api.TableEnvironment.generateRowConverterFunction(TableEnvironment.scala:721)
at org.apache.flink.table.api.StreamTableEnvironment.getConversionMapper(StreamTableEnvironment.scala:247)
at org.apache.flink.table.api.StreamTableEnvironment.translate(StreamTableEnvironment.scala:647)
```
- [x] General
- The pull request references the related JIRA issue ("[FLINK-XXX] Jira title text")
- The pull request addresses only one issue
- Each commit in the PR has a meaningful commit message (including the JIRA id)
- [ ] Documentation
- Documentation has been added for new functionality
- Old documentation affected by the pull request has been updated
- JavaDoc for public methods has been added
- [x] Tests & Build
- Functionality added by the pull request is covered by tests
- `mvn clean verify` has been executed successfully locally or a Travis build has passed
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sunjincheng121/flink FLINK-6886
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/4102.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 #4102
----
commit 3c89d1c8576ec44245ecb381be2ef35f5479c149
Author: sunjincheng121 <su...@gmail.com>
Date: 2017-06-11T05:53:15Z
[FLINK-6886][table]Fix Timestamp field can not be selected in event time case when toDataStream[T], `T` not a `Row` Type.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #4102: [FLINK-6886][table]Fix Timestamp field can not be ...
Posted by wuchong <gi...@git.apache.org>.
Github user wuchong commented on a diff in the pull request:
https://github.com/apache/flink/pull/4102#discussion_r121694007
--- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/table/api/TableEnvironment.scala ---
@@ -715,15 +715,8 @@ abstract class TableEnvironment(val config: TableConfig) {
functionName: String):
GeneratedFunction[MapFunction[Row, OUT], OUT] = {
- // validate that at least the field types of physical and logical type match
- // we do that here to make sure that plan translation was correct
- if (schema.physicalTypeInfo != inputTypeInfo) {
- throw TableException("The field types of physical and logical row types do not match." +
- "This is a bug and should not happen. Please file an issue.")
- }
-
- val fieldTypes = schema.physicalFieldTypeInfo
- val fieldNames = schema.physicalFieldNames
+ val fieldTypes = schema.logicalFieldTypeInfo
+ val fieldNames = schema.logicalFieldNames
--- End diff --
I'm not sure about this change. The physical and logical type check is to make sure all the time indicators are translated.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4102: [FLINK-6886][table]Fix Timestamp field can not be selecte...
Posted by sunjincheng121 <gi...@git.apache.org>.
Github user sunjincheng121 commented on the issue:
https://github.com/apache/flink/pull/4102
Hi @fhueske I have update the PR. according your suggestion. Please have a look at the changes.
Best,
SunJincheng
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4102: [FLINK-6886][table]Fix Timestamp field can not be selecte...
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on the issue:
https://github.com/apache/flink/pull/4102
Please see my message in the 1.3.1 thread on the dev@ list :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #4102: [FLINK-6886][table]Fix Timestamp field can not be ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/4102
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4102: [FLINK-6886][table]Fix Timestamp field can not be selecte...
Posted by sunjincheng121 <gi...@git.apache.org>.
Github user sunjincheng121 commented on the issue:
https://github.com/apache/flink/pull/4102
Hi @wuchong Thanks very much for your reviewing. According to your suggestions and our discussion, I made the following two changes:
1. Reduce ITCase, remove all the ITCase which added in `TableSoruceITCase`.
2. Replace `schema: RowSchema` param to `fieldNames: Seq[String]`.
@twalthr I appreciated if you can take look at the change #2.
Best,
SunJincheng
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4102: [FLINK-6886][table]Fix Timestamp field can not be selecte...
Posted by wuchong <gi...@git.apache.org>.
Github user wuchong commented on the issue:
https://github.com/apache/flink/pull/4102
I think the root cause is that TUMBLE_START inherits the wrong type (`TimeIndicatorRelDataType`) from rowtime column. So maybe a better solution is to re-create a `RelDataType` from `optimizedPlan`'s rowType and `orignalPlan`'s rowType and pass into the `getConversionMapper` method. I create a simple commit for this: https://github.com/wuchong/flink/commit/82c17ab45699f5f9beb925b156e760ebdeff79fb What do you think? @sunjincheng121 @fhueske
BTW, do we really need so many IT cases for this ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---