Skip to content
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

chore: decouple CLI from ROWKEY #4929

Merged
merged 5 commits into from
Mar 31, 2020
Merged

chore: decouple CLI from ROWKEY #4929

merged 5 commits into from
Mar 31, 2020

Conversation

big-andy-coates
Copy link
Contributor

@big-andy-coates big-andy-coates commented Mar 30, 2020 •
edited
Loading

Description

fixes: #4928

The server response when describing a source schema now includes fieldType='SYSTEM' or fieldType='KEY' to differentiate system columns, i.e. ROWTIME, and key columns from value column.

Old output:

 Field   | Type
-------------------------------------
 ROWTIME | BIGINT           (system)
 ROWKEY  | VARCHAR(STRING)  (system)
 IP      | VARCHAR(STRING)  (key)
 KBYTES  | BIGINT
-------------------------------------

New output:

 Field   | Type
-------------------------------------
 ROWTIME | BIGINT           (system)
 ROWKEY  | VARCHAR(STRING)  (key)
 IP      | VARCHAR(STRING)  (key)
 KBYTES  | BIGINT
-------------------------------------

The default is no field type. This design decision was taken as the FieldInfo pojo is used not just to describe the columns in the schema, but also fields in structs and a fieldType of VALUE wouldn't make much sense for a struct field. Where as no field type kind of works.

Note, if a column in the value has also been identified as a 'key' column using the WITH(KEY) syntax, then both ROWKEY and that column will be flagged with (key). However, the WITH(KEY) syntax is going v. soon, so this isn't really an issue. (#3537)

Testing done

usual + manual

Reviewer checklist

  • Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
  • Ensure relevant issues are linked (description should include text like "Fixes #")

@big-andy-coates
chore: decouple CLI from ROWKEY
18b7c79 
fixes: confluentinc#4928

The server response when describing a source schema now includes `fieldType='SYSTEM'` or `fieldType='KEY'` to differentiate system columns, i.e. ROWTIME, and key columns from value column.

The default is no field type. This design decision was taken as the `FieldInfo` pojo is used not just to describe the columns in the schema, but also fields in structs and a fieldType of `VALUE` wouldn't make much sense for a struct field. Where as no field type kind of works.
@big-andy-coates big-andy-coates requested review from JimGalasyn and a team as code owners March 30, 2020 14:40
JimGalasyn
JimGalasyn approved these changes Mar 30, 2020
View reviewed changes
Copy link
Member

@JimGalasyn JimGalasyn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thank you for the updates!

agavra
agavra approved these changes Mar 31, 2020
View reviewed changes
Copy link
Contributor

@agavra agavra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@big-andy-coates big-andy-coates merged commit 3b19f59 into confluentinc:master Mar 31, 2020
@big-andy-coates big-andy-coates deleted the decouple_cli_from_rowkey branch March 31, 2020 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agavra agavra agavra approved these changes

@JimGalasyn JimGalasyn JimGalasyn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

decouple CLI from ROWKEY
3 participants
@big-andy-coates @agavra @JimGalasyn