ORC-1409: [Docs] Add stream order description in ORC spec. #1465

deshanxiao · 2023-04-11T07:16:06Z

What changes were proposed in this pull request?

This PR is aimed to add more description about stream order in ORC spec.

Why are the changes needed?

There are many users who are misled by the order of the document table, in fact the stream has no fixed order.

How was this patch tested?

dongjoon-hyun

Please file a JIRA, @deshanxiao in Apache ORC community.
Every ASF project has its own policy and this is the way in the Apache ORC community.

wgtmac · 2023-04-14T01:46:51Z

site/specification/ORCv1.md

+Note that the order of these streams is **not fixed**. In the example
+of the integer column mentioned above, the order of the PRESENT stream
+and the DATA stream cannot be determined in advance. Instead, we need
+to determine the type of stream based on the Stream Kind, rather than


Should we mention the following:

index and data streams cannot be interleaved.

the order of streams from different columns is not fixed as well.

Thank you @wgtmac . Updated.

deshanxiao · 2023-04-19T09:00:28Z

cc @dongjoon-hyun

deshanxiao · 2023-04-26T11:26:36Z

Mark the PR as draft because I may add more details about the stream order.

deshanxiao · 2023-05-10T13:38:01Z

Hi @dongjoon-hyun @wgtmac

The PR is ready for review. Please take a look at it. Thank you~

wgtmac · 2023-05-11T02:15:35Z

site/specification/ORCv1.md

+* For a specific column type, the order of streams is **not fixed**.
+* Index and data streams cannot be **interleaved**.
+* The order of streams from different columns is **not fixed** as well.


Suggested change

* For a specific column type, the order of streams is **not fixed**.

* Index and data streams cannot be **interleaved**.

* The order of streams from different columns is **not fixed** as well.

There is a general order for index and data streams:

* Index streams are always placed together in the beginning of the stripe.

* Data streams are placed together after index streams (if any).

* Inside index streams or data streams, the unencrypted streams should be placed first and then followed by streams grouped by each encryption variant.

There is no fixed order within each unencrypted or encryption variant in the index and data streams:

* Different stream kinds of the same column can be placed in any order.

* Streams from different columns can even be placed in any order.

To get the precise information (a.k.a stream kind, column id and location) of a stream within a stripe, the streams field in the StripeFooter described below is the single source of truth.

These statements look a little bit vague to me, I have changed them to provide fixed order and flexible order. WDYT?

Also I would prefer to move them to below line 907.

Thank you @wgtmac . It has been updated.

wgtmac · 2023-05-11T03:35:43Z

site/specification/ORCv2.md

@@ -1257,6 +1277,19 @@ Because dictionaries are accessed randomly, there is not a position to
 record for the dictionary and the entire dictionary must be read even
 if only part of a stripe is being read.

+Note that for columns with multiple streams, the order of these
+streams is **fixed**, which is different from data stream we mentioned above.


Seems the doc has a line limit that we need to follow.

wgtmac · 2023-05-11T03:37:59Z

site/specification/ORCv2.md

+Note that for columns with multiple streams, the order of these
+streams is **fixed**, which is different from data stream we mentioned above.


Suggested change

Note that for columns with multiple streams, the order of these

streams is **fixed**, which is different from data stream we mentioned above.

Note that for columns with multiple streams, the order of stream

positions in the RowIndex is **fixed**, which may be different to

the actual data stream placement.

We should also mention that the order is the same as what Column Encodings section has described for each column type.

It looks resonable and will add more description/remove the chart.

wgtmac · 2023-05-11T03:40:32Z

site/specification/ORCv2.md

+The following table shows the order of types that contain multiple streams:
+
+ Type      | The First Postion Stream Type   | The Second Postion Stream Type | Contents        |
+:----------|:--------------------------------|:-------------------------------|:----------------|
+| Binary    | DATA                            | LENGTH                         |                 |
+| String    | DATA                            | LENGTH                         | Direct encoding |
+| Decimal   | DATA                            | SECONDARY                      |                 |
+| Timestamp | DATA                            | SECONDARY                      |                 |


This table is duplicated to what Column Encodings has described. And here it doesn't include PRESENT stream. I'd prefer to remove this.

wgtmac

Looking good on my side. Thanks @deshanxiao!

@dongjoon-hyun @williamhyun Do you want to check another pass?

dongjoon-hyun

+1, LGTM, too. Thank you, @deshanxiao and @wgtmac .

dongjoon-hyun · 2023-05-16T20:26:36Z

I also published to Apache ORC website.

https://orc.apache.org/specification/ORCv0/#column-encoding-section

wgtmac · 2023-05-17T05:22:48Z

Thank you @dongjoon-hyun !

### What changes were proposed in this pull request? This PR is aimed to add more description about stream order in ORC spec. ### Why are the changes needed? There are many users who are misled by the order of the document table, in fact the stream has no fixed order. apache#1450 ### How was this patch tested? Closes apache#1465 from deshanxiao/add-order-description. Authored-by: deshanxiao <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>

deshanxiao added 5 commits April 11, 2023 15:05

init

26cab9a

init

1740cf1

init

72dfc11

init'

ff929a5

init

bc125d2

github-actions bot added the DOCS label Apr 11, 2023

update

7e79a54

dongjoon-hyun reviewed Apr 11, 2023

View reviewed changes

deshanxiao changed the title ~~[Docs] Add stream order description in ORC spec.~~ ORC-1409 [Docs] Add stream order description in ORC spec. Apr 13, 2023

deshanxiao changed the title ~~ORC-1409 [Docs] Add stream order description in ORC spec.~~ ORC-1409: [Docs] Add stream order description in ORC spec. Apr 13, 2023

wgtmac reviewed Apr 14, 2023

View reviewed changes

fix comments

86fb772

deshanxiao mentioned this pull request Apr 24, 2023

[C++] Order of row index streams does not match the order of streams in the file footer #1475

Open

deshanxiao changed the title ~~ORC-1409: [Docs] Add stream order description in ORC spec.~~ ORC-1409: [Docs][Draft] Add stream order description in ORC spec. Apr 26, 2023

deshanxiao added 2 commits May 10, 2023 19:24

Merge branch 'main' into add-order-description

79b986f

add row index order

494caad

deshanxiao changed the title ~~ORC-1409: [Docs][Draft] Add stream order description in ORC spec.~~ ORC-1409: [Docs] Add stream order description in ORC spec. May 10, 2023

wgtmac reviewed May 11, 2023

View reviewed changes

change for Gang wu comments

f9d3086

wgtmac reviewed May 11, 2023

View reviewed changes

deshanxiao added 3 commits May 11, 2023 16:02

update comments

b3fe83e

add link

d3f881b

add other version link

07a6fae

wgtmac approved these changes May 11, 2023

View reviewed changes

dongjoon-hyun approved these changes May 16, 2023

View reviewed changes

dongjoon-hyun added this to the 2.0.0 milestone May 16, 2023

dongjoon-hyun closed this in 25fb755 May 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ORC-1409: [Docs] Add stream order description in ORC spec. #1465

ORC-1409: [Docs] Add stream order description in ORC spec. #1465

deshanxiao commented Apr 11, 2023 •

edited

Loading

dongjoon-hyun left a comment

wgtmac Apr 14, 2023

deshanxiao Apr 14, 2023

deshanxiao commented Apr 19, 2023

deshanxiao commented Apr 26, 2023

deshanxiao commented May 10, 2023

wgtmac May 11, 2023

wgtmac May 11, 2023

wgtmac May 11, 2023

deshanxiao May 11, 2023

wgtmac May 11, 2023

wgtmac May 11, 2023

wgtmac May 11, 2023

deshanxiao May 11, 2023

wgtmac May 11, 2023

deshanxiao May 11, 2023

wgtmac left a comment

dongjoon-hyun left a comment

dongjoon-hyun commented May 16, 2023

wgtmac commented May 17, 2023

-* For a specific column type, the order of streams is **not fixed**.
-* Index and data streams cannot be **interleaved**.
-* The order of streams from different columns is **not fixed** as well.
+There is a general order for index and data streams:
+* Index streams are always placed together in the beginning of the stripe.
+* Data streams are placed together after index streams (if any).
+* Inside index streams or data streams, the unencrypted streams should be placed first and then followed by streams grouped by each encryption variant.
+There is no fixed order within each unencrypted or encryption variant in the index and data streams:
+* Different stream kinds of the same column can be placed in any order.
+* Streams from different columns can even be placed in any order.
+To get the precise information (a.k.a stream kind, column id and location) of a stream within a stripe, the streams field in the StripeFooter described below is the single source of truth.

		Note that for columns with multiple streams, the order of these
		streams is fixed, which is different from data stream we mentioned above.

-Note that for columns with multiple streams, the order of these
-streams is **fixed**, which is different from data stream we mentioned above.
+Note that for columns with multiple streams, the order of stream
+positions in the RowIndex is **fixed**, which may be different to
+the actual data stream placement.

ORC-1409: [Docs] Add stream order description in ORC spec. #1465

ORC-1409: [Docs] Add stream order description in ORC spec. #1465

Conversation

deshanxiao commented Apr 11, 2023 • edited Loading

What changes were proposed in this pull request?

Why are the changes needed?

How was this patch tested?

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deshanxiao commented Apr 19, 2023

deshanxiao commented Apr 26, 2023

deshanxiao commented May 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wgtmac left a comment

Choose a reason for hiding this comment

dongjoon-hyun left a comment

Choose a reason for hiding this comment

dongjoon-hyun commented May 16, 2023

wgtmac commented May 17, 2023

deshanxiao commented Apr 11, 2023 •

edited

Loading