fix: Parsing for qualifiers with colon characters#1277
fix: Parsing for qualifiers with colon characters#1277danieljbruce merged 13 commits intogoogleapis:mainfrom
Conversation
|
🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use -- conventional-commit-lint bot |
|
Please fix the tests |
|
Warning: This pull request is touching the following templated files:
|
|
All done, tested locally. Apologies for the noise. Changed the qualifier assertion to check for |
src/mutation.ts
Outdated
| // columnName does not contain ':' | ||
| return { | ||
| family: columnName, | ||
| qualifier: null, |
There was a problem hiding this comment.
I recommend setting qualifier to undefined so that this code works the same way as before when the string doesn't contain a colon.
There was a problem hiding this comment.
I tried that too, however the ParsedColumn interface does not declare undefined as an acceptable type for qualifier at https://github.com/googleapis/nodejs-bigtable/blob/main/src/mutation.ts#L35.
Adding undefined here would solve this:
export interface ParsedColumn {
family: string | null;
qualifier: string | null | undefined;
}|
Consider adding another test? |
parseColumnName()uses.split(':')to find the qualifier, but this parses qualifiers with:chars in it incorrectly, since they also are picked up by the split. This causes it to ignore everything after:in the qualifier.e.g. consider a column name
"foo:bar:baz"parseColumnName()parses this incorrectly asUsing
slice()instead with the index of the first:character fixes this: