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

[PR] Add glob and single file support to BeamSQL ParquetTable LOCATION [beam]

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

   This PR modifies ParquetTable to intelligently resolve the file path before passing it to ParquetIO.
   
   It uses FileSystems.match() to check if the provided LOCATION corresponds to a directory.
   
   If it's a directory, a wildcard (*) is appended to the path to maintain the existing behavior of reading all files within it.
   
   If it's not a directory (i.e., it's a file or a path that doesn't exist yet), it's treated as a user-provided pattern (either a glob or a single file path) and is used as-is.
   
   fixes #20884 
   
   ------------------------
   
   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] Add glob and single file support to BeamSQL ParquetTable LOCATION [beam]

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


##########
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/parquet/ParquetTable.java:
##########
@@ -122,4 +124,11 @@ public IsBounded isBounded() {
   public ProjectSupport supportsProjects() {
     return ProjectSupport.WITH_FIELD_REORDERING;
   }
+
+  private String resolveFilePattern(String location) {
+    if (location.endsWith("/")) {

Review Comment:
   Previously user can provide a directory without trailing "/", now the runtime FileSystems.match() will resolve to not found or a directory (depending on filesystem type), we may mention this breaking change in CHANGES.md



-- 
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] Add glob and single file support to BeamSQL ParquetTable LOCATION [beam]

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


##########
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/parquet/ParquetTable.java:
##########
@@ -122,4 +124,11 @@ public IsBounded isBounded() {
   public ProjectSupport supportsProjects() {
     return ProjectSupport.WITH_FIELD_REORDERING;
   }
+
+  private String resolveFilePattern(String location) {
+    if (location.endsWith("/")) {

Review Comment:
   Done



-- 
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] Add glob and single file support to BeamSQL ParquetTable LOCATION [beam]

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

   Assigning reviewers:
   
   R: @chamikaramj for label java.
   
   
   Note: If you would like to opt out of this review, comment `assign to next reviewer`.
   
   Available commands:
   - `stop reviewer notifications` - opt out of the automated review tooling
   - `remind me after tests pass` - tag the comment author after tests pass
   - `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
   
   The PR bot will only process comments in the main thread (not review comments).


-- 
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] Add glob and single file support to BeamSQL ParquetTable LOCATION [beam]

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


##########
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/parquet/ParquetTable.java:
##########
@@ -122,4 +127,27 @@ public IsBounded isBounded() {
   public ProjectSupport supportsProjects() {
     return ProjectSupport.WITH_FIELD_REORDERING;
   }
+
+  private String resolveFilePattern(String location) {
+    try {
+      MatchResult match = FileSystems.match(location);
+      if (match.status() == MatchResult.Status.OK && !match.metadata().isEmpty()) {
+        MatchResult.Metadata metadata = match.metadata().get(0);
+        if (metadata.resourceId().isDirectory()) {
+          String dirPath = metadata.resourceId().toString();
+          if (dirPath.endsWith("/")) {
+            return dirPath + "*";
+          } else {
+            return dirPath + "/*";
+          }
+        }
+      }
+    } catch (IOException e) {
+      LOG.warn(

Review Comment:
   Thank you @Abacn This is good call out that i  forgot it. Let me change this function behavior in a way If location ends with "/", it's treated as a directory. Otherwise, it's treated as a glob or a single file path.
   
   ```java
     private String resolveFilePattern(String location) {
       if (location.endsWith("/")) {
         return location + "*";
       }
       return location;
     }
   ```



-- 
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] Add glob and single file support to BeamSQL ParquetTable LOCATION [beam]

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


##########
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/parquet/ParquetTable.java:
##########
@@ -122,4 +127,27 @@ public IsBounded isBounded() {
   public ProjectSupport supportsProjects() {
     return ProjectSupport.WITH_FIELD_REORDERING;
   }
+
+  private String resolveFilePattern(String location) {
+    try {
+      MatchResult match = FileSystems.match(location);
+      if (match.status() == MatchResult.Status.OK && !match.metadata().isEmpty()) {
+        MatchResult.Metadata metadata = match.metadata().get(0);
+        if (metadata.resourceId().isDirectory()) {
+          String dirPath = metadata.resourceId().toString();
+          if (dirPath.endsWith("/")) {
+            return dirPath + "*";
+          } else {
+            return dirPath + "/*";
+          }
+        }
+      }
+    } catch (IOException e) {
+      LOG.warn(

Review Comment:
   buildIOReader happens at pipeline expansion time, and it is totally valid use case that submission VM not having access to the filesystem location (consider user submit pipeline locally and run in Dataflow).
   
   How about not rely on FileSystems.match call to resolveFilePattern? For example, if location has '*', consider it as a glob; if location ends with pqt, or .parquet, consider it as a single file



-- 
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] Add glob and single file support to BeamSQL ParquetTable LOCATION [beam]

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


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