Remove the feature flag allow.table.name.with.database
#12402
Conversation
shounakmk219
changed the title
shounakmk219
changed the title
allow.table.name.with.database
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #12402 +/- ##
============================================
- Coverage 61.73% 61.71% -0.02%
Complexity 207 207
============================================
Files 2428 2428
Lines 132828 132823 -5
Branches 20545 20544 -1
============================================
- Hits 82007 81978 -29
- Misses 44811 44838 +27
+ Partials 6010 6007 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Jackie-Jiang
added
the
release-notes
| @@ -91,8 +91,6 @@ public static class Helix { | |||
|
|
|||
| public static final String ENABLE_CASE_INSENSITIVE_KEY = "enable.case.insensitive"; | |||
| public static final boolean DEFAULT_ENABLE_CASE_INSENSITIVE = true; | |||
| public static final String ALLOW_TABLE_NAME_WITH_DATABASE = "allow.table.name.with.database"; | |||
| public static final boolean DEFAULT_ALLOW_TABLE_NAME_WITH_DATABASE = false; | |||
There was a problem hiding this comment.
Should we consider keeping this config but only change the default to true? It might be useful for certain cases
There was a problem hiding this comment.
Yeah even I thought about that but as part of introducing database we will have lot of things like APIs, access control that's database aware so toggling the behaviour everywhere based on the feature flag will be complicated.
Do you have a strong use-case in mind which needs banning dot in table name?
There was a problem hiding this comment.
Going through the code again, seems we only looses the table name check by always allowing one dot. I think it should be fine
| @@ -91,8 +91,6 @@ public static class Helix { | |||
|
|
|||
| public static final String ENABLE_CASE_INSENSITIVE_KEY = "enable.case.insensitive"; | |||
| public static final boolean DEFAULT_ENABLE_CASE_INSENSITIVE = true; | |||
| public static final String ALLOW_TABLE_NAME_WITH_DATABASE = "allow.table.name.with.database"; | |||
| public static final boolean DEFAULT_ALLOW_TABLE_NAME_WITH_DATABASE = false; | |||
There was a problem hiding this comment.
Going through the code again, seems we only looses the table name check by always allowing one dot. I think it should be fine
gortiz
added
the
backward-incompat
Description
Removing the feature flag introduced in #8713 and allowing table name with
.throughout in favour of supportingdatabaseconcept in Pinot (discussed in issue #12333 ).Release Notes
allow.table.name.with.databasecluster config is removed