[GitHub] flink issue #4111: [FLINK-6896][table] Fix generate PojoType input result ex...

2017-06-19 Thread wuchong
Github user wuchong commented on the issue:

https://github.com/apache/flink/pull/4111
  
Of course @twalthr , I haven't merged the code and thanks for your new PR. 
I will have a look tomorrow.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #4111: [FLINK-6896][table] Fix generate PojoType input result ex...

2017-06-19 Thread twalthr
Github user twalthr commented on the issue:

https://github.com/apache/flink/pull/4111
  
@sunjincheng121 I opened #4144. Feel free to review and test. I really hope 
that I could cover all cases.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #4111: [FLINK-6896][table] Fix generate PojoType input result ex...

2017-06-19 Thread sunjincheng121
Github user sunjincheng121 commented on the issue:

https://github.com/apache/flink/pull/4111
  
Hi @twalthr Sounds good, And make sense to me.  I think i can do some code 
review or check some test case when you opened the PR. :)

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #4111: [FLINK-6896][table] Fix generate PojoType input result ex...

2017-06-19 Thread twalthr
Github user twalthr commented on the issue:

https://github.com/apache/flink/pull/4111
  
@sunjincheng121 Sorry, that I haven't responded earlier. I was on vacation 
last week. I found a solution that requires less CodeGenerator changes. I found 
several issues (also regarding Java expression parsing). I will open a PR 
shortly. We can test the PR, with the test cases of this PR.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #4111: [FLINK-6896][table] Fix generate PojoType input result ex...

2017-06-19 Thread sunjincheng121
Github user sunjincheng121 commented on the issue:

https://github.com/apache/flink/pull/4111
  
Hi @twalthr I have test this case using the first approach of FLINK-6896. 
without `as` clause. It's works well. And with the `as` clause, It can be fixed 
by add a case match in 
`StreamTableEnvironment#validateAndExtractTimeAttributes`. the code as follows:
`case (Alias(child, name,_), _) => fieldNames = name :: fieldNames` What do 
you think?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #4111: [FLINK-6896][table] Fix generate PojoType input result ex...

2017-06-19 Thread sunjincheng121
Github user sunjincheng121 commented on the issue:

https://github.com/apache/flink/pull/4111
  
@twalthr Thank you for pay attention to this PR. Feel free to left your 
comments.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #4111: [FLINK-6896][table] Fix generate PojoType input result ex...

2017-06-19 Thread rmetzger
Github user rmetzger commented on the issue:

https://github.com/apache/flink/pull/4111
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #4111: [FLINK-6896][table] Fix generate PojoType input result ex...

2017-06-19 Thread twalthr
Github user twalthr commented on the issue:

https://github.com/apache/flink/pull/4111
  
@wuchong Can we wait one more day before merging this? I also want to take 
a look at it. It is related to FLINK-6881.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #4111: [FLINK-6896][table] Fix generate PojoType input result ex...

2017-06-15 Thread wuchong
Github user wuchong commented on the issue:

https://github.com/apache/flink/pull/4111
  
Travis reported a bug about composite type. Fixed it. Waiting for the new 
CI pass.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #4111: [FLINK-6896][table] Fix generate PojoType input result ex...

2017-06-14 Thread sunjincheng121
Github user sunjincheng121 commented on the issue:

https://github.com/apache/flink/pull/4111
  
Thanks for your changes. @fhueske 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #4111: [FLINK-6896][table] Fix generate PojoType input result ex...

2017-06-14 Thread hustfxj
Github user hustfxj commented on the issue:

https://github.com/apache/flink/pull/4111
  
Looks good to me.
+1 to merged.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #4111: [FLINK-6896][table] Fix generate PojoType input result ex...

2017-06-14 Thread hustfxj
Github user hustfxj commented on the issue:

https://github.com/apache/flink/pull/4111
  
@sunjincheng121  I get it. Thank you very much!


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #4111: [FLINK-6896][table] Fix generate PojoType input result ex...

2017-06-14 Thread sunjincheng121
Github user sunjincheng121 commented on the issue:

https://github.com/apache/flink/pull/4111
  
For example (from test case):
You can run 
`org.apache.flink.table.api.java.batch.TableEnvironmentITCase#testAsFromAndToPrivateFieldPojo`

* input1:

```
0=age(Integer)
1=department(String)
2=name(String)
3=(Double)
```

* input1Mapping:

```
0=1
1=0
2=3
3=2
```

Using this PR code logic,we get:

input1AccessExprs:

```
0=String
1=Integer
2=Double
3=String
```
If Using `for (i <- 0 until input1.getArity if input1Mapping.contains(i))` 
and delete you mentioned code.we get:
input1AccessExprs:

```
0=Integer
1=String
2=String
3=Double
```
That not we want, then the following code will throw exception:
  `Incompatible types of expression and result type.`

I try to express my point clearly. But feel free to tell me If there 
something not enough.

Thanks,
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #4111: [FLINK-6896][table] Fix generate PojoType input result ex...

2017-06-14 Thread hustfxj
Github user hustfxj commented on the issue:

https://github.com/apache/flink/pull/4111
  
@sunjincheng121  Sorry, I didn't get you. I think the column will not be 
out of order If we delete the code. Because we already ensure that the index is 
at the correct range by the  previous 
 code `input1Mapping.contains(i)`. Let me know if I'm wrong. Thank you!


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #4111: [FLINK-6896][table] Fix generate PojoType input result ex...

2017-06-14 Thread sunjincheng121
Github user sunjincheng121 commented on the issue:

https://github.com/apache/flink/pull/4111
  
Hi @hustfxj Thanks for your review. If I understand you correctly. you want 
delete the code which you mentioned above. But I'm afraid we can not delete 
that code. Because when output type is `PojoTypeInfo`, we really need that 
code. Can you double check it And Please feedback me?

Thanks,
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #4111: [FLINK-6896][table] Fix generate PojoType input result ex...

2017-06-14 Thread hustfxj
Github user hustfxj commented on the issue:

https://github.com/apache/flink/pull/4111
  
@sunjincheng121  Thank you for the Fix. Maybe  it had better remove the 
followed codes at the Func   **generateFieldAccess**.
```
val fieldIndex = if (ct.isInstanceOf[PojoTypeInfo[_]]) {
  fieldMapping(index)
}
``` 
  And this bug was import by  [FLINK-5884], due to  the judgment 
**input1Mapping** contains the field index or not at the Func 
**generateConverterResultExpression**.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---