vampire-hunter-lincoln commented on issue #20865:
URL: https://github.com/apache/beam/issues/20865#issuecomment-3499252489

   
   Hi @damccorm,
   
   **Context**
   My team is using [logical 
DATE](https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/logicaltypes/SqlTypes.java#L33)(`LogicalType<LocalDate,
 Long>`) to represent `DATE` in Beam SQL(backed by Apache Calcite), but I find 
[this 
comment](https://github.com/Abacn/beam/blob/master/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/rel/BeamCalcRel.java#L422).
 
   
   **Analysis**
   After reading some related source code, here is what happened for logical 
DATE(`LogicalType<LocalDate, Long>`) processing in Beam SQL according to my 
understanding, please correct me if I am wrong:
   1. Since Beam configures Calcite to use java.lang.Long as the physical type 
for Calcite DATE, it means the compiled code will expect a Long object to hold 
value of Calcite DATE. Janino, as a Java compiler, will compile the expression 
according to Java's type rules.
   2. During `Beam value` -> `Calcite Value`, an Integer value is extracted 
from the Java 
LocalDate([code](https://github.com/Abacn/beam/blob/master/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/rel/BeamCalcRel.java#L668)).
 We provide this integer as a Calcite DATE input in [Janino evaluation 
step](https://github.com/Abacn/beam/blob/master/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/rel/BeamCalcRel.java#L317).
 
   3. During Janino evaluation step, if any conversion(expression) happens to 
this Integer, the result will be saved into a Long object and return a Long as 
output. Otherwise, it will still be the same Integer as output.  That can 
explain the [original 
comment](https://github.com/Abacn/beam/blob/master/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/rel/BeamCalcRel.java#L422).
   
   **Solution**
   So I believe, we can modify the step 2 above, extract a Long value from the 
Java LocalDate during `Beam value` -> `Calcite Value`(those two lines: 
[line1](https://github.com/apache/beam/blob/master/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/rel/BeamCalcRel.java#L671)
 and 
[line2](https://github.com/apache/beam/blob/master/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/rel/BeamCalcRel.java#L681)).
 Then everything will be consistent as Long since both the value and definition 
will be Long.
   
   **Notes**
   This  issue is same as  https://github.com/apache/beam/issues/20864
   
   Please let me know if you have any thoughts! Thank you!
   
   CC: @Abacn 


-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to