Skip to content

Fix table lookup for FileSystemCatalog with S3 path - #1411

Merged
scott-routledge2 merged 6 commits into
mainfrom
scott/add_bodosql_missing_aws_depedencies
Sep 24, 2026
Merged

scott-routledge2 merged 6 commits into
mainfrom
scott/add_bodosql_missing_aws_depedencies

Conversation

@scott-routledge2

@scott-routledge2 scott-routledge2 commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Changes included in this PR

  • Add missing AWS SDK dependencies
  • Fix catalog path handling in C++ backend, plan conversion
  • Fix empty schemas leading in TPCH table paths

Testing strategy

New test added:

from bodosql import FileSystemCatalog, BodoSQLContext

catalog = FileSystemCatalog("s3://.../tpch_sf10_iceberg")
bc = BodoSQLContext(catalog=catalog)

print(bc.sql("SELECT * FROM NATION"))

User facing changes

Checklist

  • PR title contains "[GPU]" if changes target Bodo DataFrames GPU acceleration.
  • Pipelines passed before requesting review. To run CI you must include [run CI] in your commit message.
  • I am familiar with the Contributing Guide
  • I have installed + ran pre-commit hooks.

@IsaacWarren IsaacWarren left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Scott


<dependency>
<groupId>com.amazonaws</groupId>
<artifactId>aws-java-sdk-dynamodb</artifactId>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need dynamodb?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure, I saw the error after adding s3.

I'll investigate a bit more.

@scott-routledge2 scott-routledge2 Sep 24, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the error, it looks like dynamo might be a dependency of hadoop.fs.s3a.S3AFileSystem.

And we are seeing these errors in the first place because we are explicitly excluding the aws-java-sdk-bundled dependency--

I am assuming this is to keep our JAR size small?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's for jar size

@scott-routledge2 scott-routledge2 changed the title Add missing AWS SDK depedendencies for FileSystemCatalog Fix table lookup for FileSystemCatalog with S3 path Sep 24, 2026
@codecov

codecov Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.16%. Comparing base (0f7e335) to head (689f11e).
⚠️ Report is 52 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1411      +/-   ##
==========================================
+ Coverage   66.55%   68.16%   +1.61%     
==========================================
  Files         198      198              
  Lines       68731    68998     +267     
  Branches     9834     9908      +74     
==========================================
+ Hits        45744    47034    +1290     
+ Misses      20071    19089     -982     
+ Partials     2916     2875      -41     

@scott-routledge2
scott-routledge2 marked this pull request as ready for review September 24, 2026 21:15

@DrTodd13 DrTodd13 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@scott-routledge2
scott-routledge2 merged commit 3b4f66d into main Sep 24, 2026
16 of 17 checks passed
@scott-routledge2
scott-routledge2 deleted the scott/add_bodosql_missing_aws_depedencies branch September 24, 2026 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants