Skip to content

Format: Cleanup language in spec ORC type notes#4976

Merged
rdblue merged 1 commit intoapache:masterfrom
szehon-ho:doc_orc
Jun 29, 2022
Merged

Format: Cleanup language in spec ORC type notes#4976
rdblue merged 1 commit intoapache:masterfrom
szehon-ho:doc_orc

Conversation

@szehon-ho
Copy link
Member

@szehon-ho szehon-ho commented Jun 6, 2022

Clean up some rough language in the user-facing spec documentation.


Notes:

1. ORC's [TimestampColumnVector](https://orc.apache.org/api/hive-storage-api/org/apache/hadoop/hive/ql/exec/vector/TimestampColumnVector.html) comprises of a time field (milliseconds since epoch) and a nanos field (nanoseconds within the second). Hence the milliseconds within the second are reported twice; once in the time field and again in the nanos field. The read adapter should only use milliseconds within the second from one of these fields. The write adapter should also report milliseconds within the second twice; once in the time field and again in the nanos field. ORC writer is expected to correctly consider millis information from one of the fields. More details at https://issues.apache.org/jira/browse/ORC-546
Copy link
Member Author

@szehon-ho szehon-ho Jun 6, 2022

Choose a reason for hiding this comment

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

Explanation: "comprise of" is apparently not gramatically correct, it's actually the other way : "a time field and nano field comprise ORC's TimestampColumnVector". This was from my editor's auto-correct which suggested to just use 'consists of' instead.

@szehon-ho szehon-ho changed the title Docs: Cleanup language in spec ORC type notes Format: Cleanup language in spec ORC type notes Jun 7, 2022
@rdblue rdblue merged commit 2dab406 into apache:master Jun 29, 2022
@rdblue
Copy link
Contributor

rdblue commented Jun 29, 2022

Thanks, @szehon-ho! Good catch.

namrathamyske pushed a commit to namrathamyske/iceberg that referenced this pull request Jul 10, 2022
namrathamyske pushed a commit to namrathamyske/iceberg that referenced this pull request Jul 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments