You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "Abacn (via GitHub)" <gi...@apache.org> on 2025/07/30 17:19:06 UTC

[PR] Fix BeamSQL CalcRel DATETIME expression [beam]

Abacn opened a new pull request, #35743:
URL: https://github.com/apache/beam/pull/35743

   Fix #35736
   
   **Please** add a meaningful description for your change here
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
    - [ ] Update `CHANGES.md` with noteworthy changes.
    - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://github.com/apache/beam/blob/master/CONTRIBUTING.md#make-the-reviewers-job-easier).
   
   To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
   
   GitHub Actions Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   [![Build python source distribution and wheels](https://github.com/apache/beam/actions/workflows/build_wheels.yml/badge.svg?event=schedule&&?branch=master)](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
   [![Python tests](https://github.com/apache/beam/actions/workflows/python_tests.yml/badge.svg?event=schedule&&?branch=master)](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Java tests](https://github.com/apache/beam/actions/workflows/java_tests.yml/badge.svg?event=schedule&&?branch=master)](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Go tests](https://github.com/apache/beam/actions/workflows/go_tests.yml/badge.svg?event=schedule&&?branch=master)](https://github.com/apache/beam/actions?query=workflow%3A%22Go+tests%22+branch%3Amaster+event%3Aschedule)
   
   See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI or the [workflows README](https://github.com/apache/beam/blob/master/.github/workflows/README.md) to see a list of phrases to trigger workflows.
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix BeamSQL CalcRel DATETIME expression [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35743:
URL: https://github.com/apache/beam/pull/35743#issuecomment-3137218787

   Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment `assign set of reviewers`


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix BeamSQL CalcRel DATETIME expression [beam]

Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on code in PR #35743:
URL: https://github.com/apache/beam/pull/35743#discussion_r2243384371


##########
sdks/java/extensions/sql/src/test/java/org/apache/beam/sdk/extensions/sql/BeamComplexTypeTest.java:
##########
@@ -349,6 +350,32 @@ public void testNestedArrayOfBytes() {
     pipeline.run();
   }
 
+  @Test
+  public void testNestedDatetime() {
+    List<Instant> dateTimes =
+        ImmutableList.of(Instant.EPOCH, Instant.ofEpochSecond(10000), Instant.now());

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   Using `Instant.now()` in test data can lead to non-deterministic tests if the test logic changes in the future. It's better to use a fixed instant for reproducibility, even if this value is not directly used in the assertion.
   
   ```suggestion
           ImmutableList.of(Instant.EPOCH, Instant.ofEpochSecond(10000), Instant.ofEpochSecond(20000));
   ```



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix BeamSQL CalcRel DATETIME expression [beam]

Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on PR #35743:
URL: https://github.com/apache/beam/pull/35743#issuecomment-3137215581

   R: @ahmedabu98 @vampire-hunter-lincoln


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix BeamSQL CalcRel DATETIME expression [beam]

Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on code in PR #35743:
URL: https://github.com/apache/beam/pull/35743#discussion_r2245707299


##########
sdks/java/extensions/sql/src/test/java/org/apache/beam/sdk/extensions/sql/BeamComplexTypeTest.java:
##########
@@ -349,6 +350,32 @@ public void testNestedArrayOfBytes() {
     pipeline.run();
   }
 
+  @Test
+  public void testNestedDatetime() {
+    List<Instant> dateTimes =
+        ImmutableList.of(Instant.EPOCH, Instant.ofEpochSecond(10000), Instant.now());

Review Comment:
   Added nullable array field to exercise this case



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix BeamSQL CalcRel DATETIME expression [beam]

Posted by "vampire-hunter-lincoln (via GitHub)" <gi...@apache.org>.
vampire-hunter-lincoln commented on code in PR #35743:
URL: https://github.com/apache/beam/pull/35743#discussion_r2244307319


##########
sdks/java/extensions/sql/src/test/java/org/apache/beam/sdk/extensions/sql/BeamComplexTypeTest.java:
##########
@@ -349,6 +350,32 @@ public void testNestedArrayOfBytes() {
     pipeline.run();
   }
 
+  @Test
+  public void testNestedDatetime() {
+    List<Instant> dateTimes =
+        ImmutableList.of(Instant.EPOCH, Instant.ofEpochSecond(10000), Instant.now());

Review Comment:
   `ImmutableList` cannot contain `null` element, right? Can we add one more test case to verify this change can support a list of Joda Instant with some null elements?



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix BeamSQL CalcRel DATETIME expression [beam]

Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn merged PR #35743:
URL: https://github.com/apache/beam/pull/35743


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org