fix: make stream and column names case-insensitive in /inserts-stream #5591
Conversation
| @@ -97,6 +97,20 @@ public static String convertCommaSeparatedWilcardsToRegex(final String csv) { | |||
| return out.toString(); | |||
| } | |||
|
|
|||
| // See ParserUtil#getIdentifierText() | |||
| public static String getIdentifierText(final String text) { | |||
There was a problem hiding this comment.
It's not great that this logic is repeated here and also in the method of the same name in ParserUtil.java. I couldn't think of a way to combine them, though, since the ParserUtil method is specific to the AST.
| @@ -69,6 +71,28 @@ public static GenericRow extractValues(final JsonObject values, final LogicalSch | |||
| return GenericRow.fromList(vals); | |||
| } | |||
|
|
|||
| static JsonObject convertColumnNameCase(final JsonObject jsonObjectWithCaseInsensitiveFields) { | |||
There was a problem hiding this comment.
Instead of iterating through the entries again in here and having an extra method, it would be simpler to call ServerUtils.getIdentifierText in the existing extractValues method.
There was a problem hiding this comment.
Then we'd also have to do the conversion in extractKey() too, right? I think it's better to do it once upfront, rather than making both extractValues() and extractKey() responsible for converting.
I'm going to merge this for now; can open a follow-up with the requested changes if I've misunderstood.
| } | ||
|
|
||
| final char firstChar = text.charAt(0); | ||
| if (firstChar == '`' || firstChar == '"') { |
There was a problem hiding this comment.
Do we really also support double quotes for escaping? Thought it was just backtick.
There was a problem hiding this comment.
We also support double quotes. Here's the relevant code:
I also sanity checked manually.
| public void shouldTreatInsertColumnNamesAsCaseSensitiveIfQuoted() { | ||
| // Given: | ||
| JsonObject row = new JsonObject() | ||
| .put("\"str\"", "HELLO") |
|
As discussed in #5588 (review), I'm going to intentionally leave out the breaking note change in this PR description from the final commit message (so it won't appear in the auto-generated changelog). |
Description
The stream name and column names provided to the
/inserts-streamendpoint are both currently case-sensitive, meaning an attempted insert into a stream with namefoowill fail if the stream actually has nameFOOin the metastore (e.g., if the stream were created withcreate stream foo ...). The same holds for column names as well. This PR updates the/inserts-streamendpoint to be case-insensitive by default, in order to be consistent with the rest of ksqlDB.BREAKING CHANGE: Stream name and column names provided to the
/inserts-streamendpoint are no longer case-sensitive. Instead, they will be upper-cased by default. To preserve case-sensitivity, surround the names with double-quotes or backticks.Testing done
Added integration tests.
Reviewer checklist