Skip to content

Database name validation for logical tables #15994

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

abhishekbafna
Copy link
Contributor

@abhishekbafna abhishekbafna commented Jun 4, 2025

The database name validation for the logical tables.

  • The database name should be same for logical table and all referenced physical tables.
  • Apply database from headers to logical table name and all referenced physical tables uniformly.

Closes: #15988

@abhishekbafna abhishekbafna changed the title Database name validation for logical tables. Database name validation for logical tables Jun 4, 2025
@codecov-commenter
Copy link

codecov-commenter commented Jun 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.39%. Comparing base (1a476de) to head (761911a).
Report is 198 commits behind head on master.

❌ Unsupported file format

Upload processing failed due to unsupported file format. Please review the parser error message:

Error parsing JUnit XML in /home/runner/work/pinot/pinot/pinot-query-planner/target/surefire-reports/TEST-org.apache.pinot.query.queries.ResourceBasedQueryPlansTest.xml at 142:1103

Caused by:
    RuntimeError: Error converting computed name to ValidatedString
    
    Caused by:
        string is too long

For more help, visit our troubleshooting guide.

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #15994      +/-   ##
============================================
+ Coverage     62.90%   63.39%   +0.49%     
+ Complexity     1386     1357      -29     
============================================
  Files          2867     2910      +43     
  Lines        163354   166899    +3545     
  Branches      24952    25528     +576     
============================================
+ Hits         102755   105806    +3051     
- Misses        52847    53078     +231     
- Partials       7752     8015     +263     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-11 63.36% <100.00%> (+0.49%) ⬆️
java-21 63.36% <100.00%> (+0.54%) ⬆️
skip-bytebuffers-false ?
skip-bytebuffers-true ?
temurin 63.39% <100.00%> (+0.49%) ⬆️
unittests 63.39% <100.00%> (+0.49%) ⬆️
unittests1 56.46% <0.00%> (+0.64%) ⬆️
unittests2 33.46% <100.00%> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vrajat
Copy link
Collaborator

vrajat commented Jun 5, 2025

There is another condition that has to be enforced.

  • Database header enforces a specific database. Default is default
  • It should not be possible to reference database objects from a database different from the one specified in the header.

For example,

curl -H "database: db" 

should not be allowed to execute CRUD APIs or query a logical table db2.logical.

IIUC that is not enforced in this PR ?

@vrajat
Copy link
Collaborator

vrajat commented Jun 5, 2025

why did codecov fail ? Is it specific to this PR ?

@abhishekbafna
Copy link
Contributor Author

"database: db"

@vrajat this is already enforced. We have tests as well for this.

public void testLogicalTableDatabaseHeaderMismatchValidation() {

@abhishekbafna
Copy link
Contributor Author

why did codecov fail ? Is it specific to this PR ?

idk, I can push another commit either for any review comment or master rebase so that it will be triggered again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Same database validation for logical table and reference physical tables
3 participants