-
Notifications
You must be signed in to change notification settings - Fork 25
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.
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
Logical type : TIME #143
Logical type : TIME #143
Conversation
value: string | bigint | number; | ||
unit: 'MILLIS' | 'MICROS' | 'NANOS'; | ||
isAdjustedToUTC: boolean; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type for handling incoming time type and handle accordingly
if (!fieldValue.items.properties.unit.enum) { | ||
throw new UnsupportedJsonSchemaError('Unit enum is not defined'); | ||
} | ||
const unit = fieldValue.items.properties.unit.default || fieldValue.items.properties.unit.enum[0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A question here, how will units be defined in schema, an enum? or plain string. if enum do we initialize all unit types as separate fields?
"unit": { | ||
"type": "string", | ||
"enum": ["MILLIS", "MICROS", "NANOS"], | ||
"description": "The unit for the time value" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How we expect units be defined a string or enum?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fine with the enum or string, so what it is now is good by me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the tests.
Problem
Part of #99
Support logical types in parquetjs starting with
TIME
supportSolution
Implementation following the parquet spec
Change summary:
Steps to Verify: