Skip to content

Reject property names with dot #17241

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.

Sign up for GitHub

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

Jump to bottom
Merged
findepi merged 4 commits into from
Apr 28, 2023
Merged

Reject property names with dot #17241

findepi merged 4 commits into from
Apr 28, 2023

Conversation

@findepi
Copy link
Member

@findepi findepi commented Apr 26, 2023

No description provided.

findepi added 2 commits April 26, 2023 10:51
@findepi
Fix typo in Delta session property name
7878675
@findepi
Reject property names with dot
f51fd08 
Property name cannot contain a dot. This is especially true for session
property names transferred as HTTP headers, but applies to schema, table
and column properties as well. Properties with a dot were not usable.
@cla-bot cla-bot bot added the cla-signed label Apr 26, 2023
@findepi
Copy link
Member Author

findepi commented Apr 26, 2023

thanks @wweiss-starburst for reporting this

findepi added 2 commits April 26, 2023 12:59
@findepi
Rename test helper classes to indicate they are not tests
1cfaac2
@findepi
Test BigQuery plugin initialization
8975aad 
Add a unit test for BigQuery plugin setup. The credentials key is
verbatim copy from product test environment.
@github-actions github-actions bot added bigquery BigQuery connector delta-lake Delta Lake connector labels Apr 26, 2023
@findepi findepi force-pushed the findepi/reject-property-names-with-dot-c3199b branch from fe5940f to 102e5aa Compare April 27, 2023 09:40
@findepi
Copy link
Member Author

findepi commented Apr 27, 2023

test-other-modules -- #17158

@findepi findepi force-pushed the findepi/reject-property-names-with-dot-c3199b branch from 102e5aa to 8975aad Compare April 28, 2023 12:11
@findepi findepi merged commit cce27a9 into trinodb:master Apr 28, 2023
@findepi findepi deleted the findepi/reject-property-names-with-dot-c3199b branch April 28, 2023 19:57
@github-actions github-actions bot added this to the 416 milestone Apr 28, 2023
@colebow colebow mentioned this pull request May 3, 2023
Merged
@colebow
Copy link
Member

colebow commented May 3, 2023

Does this need release notes? I don't think so, but want to be sure... @findepi

@findepi
Copy link
Member Author

findepi commented May 3, 2023

it does not

@findepi findepi added the no-release-notes This pull request does not require release notes entry label May 3, 2023
@piotrrzysko
Copy link
Member

piotrrzysko commented Aug 23, 2024

Do you remember what problems properties with dots were causing, besides the misspelled property fixed in this PR?

I'm considering using the PropertyMetadata class to represent connectors' properties. They contain dots. The plan is to expose these properties from ConnectorFactory and then use them in the engine for processing (validation and masking security-sensitive information) their values entered by the user through SQL.

@losipiuk
Copy link
Member

losipiuk commented Aug 23, 2024

Do you remember what problems properties with dots were causing, besides the misspelled property fixed in this PR?

I'm considering using the PropertyMetadata class to represent connectors' properties. They contain dots. The plan is to expose these properties from ConnectorFactory and then use them in the engine for processing (validation and masking security-sensitive information) their values entered by the user through SQL.

For catalog session property the syntax is:

SET SESSION catalog_name.session_property = ....

For global session properties the syntax is:

SET SESSION session_property = ....

Now with global session property with dot we have amiguity.

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

Reviewers

@losipiuk losipiuk losipiuk approved these changes

@ebyhr ebyhr ebyhr approved these changes

@electrum electrum Awaiting requested review from electrum

@martint martint Awaiting requested review from martint

@kokosing kokosing Awaiting requested review from kokosing

@hashhar hashhar Awaiting requested review from hashhar

Assignees

No one assigned

Labels

bigquery BigQuery connector cla-signed delta-lake Delta Lake connector no-release-notes This pull request does not require release notes entry

Milestone

416

Development

Successfully merging this pull request may close these issues.

5 participants

@findepi @colebow @piotrrzysko @losipiuk @ebyhr