You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "bkietz (via GitHub)" <gi...@apache.org> on 2023/09/01 19:33:21 UTC
[GitHub] [arrow] bkietz opened a new pull request, #37526: GH-35627: [Format][Integration] Add string-view to arrow format
bkietz opened a new pull request, #37526:
URL: https://github.com/apache/arrow/pull/37526
String view (and equivalent non-utf8 binary view) is an alternative representation for
variable length strings which offers greater efficiency for several common operations.
This representation is in use by UmbraDB, DuckDB, and Velox. Where those databases use
a raw pointer to out-of-line strings this PR uses a pair of 32 bit integers as a
buffer index and offset, which
- makes explicit the guarantee that lifetime of all character data is equal
to that of the array which views it, which is critical for confident
consumption across an interface boundary
- makes the arrays meaningfully serializable and
venue agnostic; directly usable in shared memory without modification
- allows easy validation
This PR is extracted from https://github.com/apache/arrow/pull/35628 to unblock independent PRs now that the vote has passed, including:
- New types added to Schema.fbs
- Message.fbs amended to support variable buffer counts between string view chunks
- datagen.py extended to produce integration JSON for string view arrays
- Columnar.rst amended with a description of the string view format
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1330507924
##########
docs/source/format/Columnar.rst:
##########
@@ -350,6 +352,53 @@ will be represented as follows: ::
|----------------|-----------------------|
| joemark | unspecified (padding) |
+.. versionadded:: Arrow Columnar Format 1.4
+
+Variable-size Binary View Layout
+--------------------------------
+
+Each value in this layout consists of 0 or more bytes. These characters'
+locations are indicated using a **views** buffer, which may point to one
+of potentially several **data** buffers or may contain the characters
+inline.
+
+The views buffer contains `length` view structures with the following layout:
+
+::
+
+ * Short strings, length <= 12
+ | Bytes 0-3 | Bytes 4-15 |
+ |------------|---------------------------------------|
+ | length | data (padded with 0) |
+
+ * Long strings, length > 12
+ | Bytes 0-3 | Bytes 4-7 | Bytes 8-11 | Bytes 12-15 |
+ |------------|------------|------------|-------------|
+ | length | prefix | buf. index | offset |
+
+In both the long and short string cases, the first four bytes encode the
+length of the string and can be used to determine how the rest of the view
+should be interpreted.
+
+In the short string case the string's bytes are inlined- stored inside the
+view itself, in the twelve bytes which follow the length.
+
+In the long string case, a buffer index indicates which character buffer
+stores the characters and an offset indicates where in that buffer the
+characters begin. Buffer index 0 refers to the first character buffer, IE
+the first buffer **after** the validity buffer and the views buffer.
+The half-open range ``[offset, offset + length)`` must be entirely contained
+within the indicated buffer. A copy of the first four bytes of the string is
+stored inline in the prefix, after the length. This prefix enables a
+profitable fast path for string comparisons, which are frequently determined
+within the first four bytes.
+
+Views must be aligned to an 8-byte boundary. This restriction enables more
+efficient interoperation with systems where the index and offset are replaced
+by a raw pointer. All integers (length, buffer index, and offset) are unsigned
Review Comment:
https://lists.apache.org/thread/x3mh2zh5qlm3fss2716yn0dkortz6z3w
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] wesm commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "wesm (via GitHub)" <gi...@apache.org>.
wesm commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323349402
##########
format/Message.fbs:
##########
@@ -99,6 +99,14 @@ table RecordBatch {
/// Optional compression of the message body
compression: BodyCompression;
+
+ /// Some types such as Utf8View are represented using a variable number of buffers.
+ /// For each such Field in the pre-ordered flattened logical schema, there will be
Review Comment:
I agree it is not strictly clear from reading
##########
docs/source/format/Columnar.rst:
##########
@@ -106,8 +106,10 @@ the different physical layouts defined by Arrow:
* **Primitive (fixed-size)**: a sequence of values each having the
same byte or bit width
* **Variable-size Binary**: a sequence of values each having a variable
- byte length. Two variants of this layout are supported using 32-bit
- and 64-bit length encoding.
+ byte length. Three variants of this layout are supported using
+ * 32-bit offset encoding
+ * 64-bit offset encoding
+ * 128-bit view-or-inline encoding
Review Comment:
I can see both sides of the argument. From the standpoint of "it encodes the same type of data" (a variable-size binary value) then including it in this list makes sense, and is perhaps somewhat less confusing for a reader of the specification.
##########
docs/source/format/Columnar.rst:
##########
@@ -350,6 +352,53 @@ will be represented as follows: ::
|----------------|-----------------------|
| joemark | unspecified (padding) |
+.. versionadded:: Arrow Columnar Format 1.4
+
+Variable-size Binary View Layout
Review Comment:
I would agree that this should at least be made consistent with the decision made above (perhaps just added as a subsection within Variable-size Binary).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323135428
##########
format/Schema.fbs:
##########
@@ -171,6 +172,24 @@ table LargeUtf8 {
table LargeBinary {
}
+/// Same as Utf8, but string characters are delimited with a packed
+/// length/pointer instead of offsets.
+
Review Comment:
Oops, I thought I'd deleted this.
##########
format/Schema.fbs:
##########
@@ -171,6 +172,24 @@ table LargeUtf8 {
table LargeBinary {
}
+/// Same as Utf8, but string characters are delimited with a packed
+/// length/pointer instead of offsets.
+
Review Comment:
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323375274
##########
format/Message.fbs:
##########
@@ -99,6 +99,14 @@ table RecordBatch {
/// Optional compression of the message body
compression: BodyCompression;
+
+ /// Some types such as Utf8View are represented using a variable number of buffers.
+ /// For each such Field in the pre-ordered flattened logical schema, there will be
Review Comment:
I'll add an example to the comment
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on PR #37526:
URL: https://github.com/apache/arrow/pull/37526#issuecomment-1727989993
CI failures [1](https://github.com/apache/arrow/actions/runs/6250641589/job/16969947670?pr=37526), [2](https://github.com/apache/arrow/actions/runs/6250641589/job/16969948099?pr=37526) are unrelated, see https://github.com/apache/arrow/issues/37803
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1317962984
##########
docs/source/format/Columnar.rst:
##########
@@ -350,6 +352,53 @@ will be represented as follows: ::
|----------------|-----------------------|
| joemark | unspecified (padding) |
+.. versionadded:: Arrow Columnar Format 1.4
+
+Variable-size Binary View Layout
+--------------------------------
+
+Each value in this layout consists of 0 or more bytes. These characters'
+locations are indicated using a **views** buffer, which may point to one
+of potentially several **data** buffers or may contain the characters
+inline.
+
+The views buffer contains `length` view structures with the following layout:
+
+::
+
+ * Short strings, length <= 12
+ | Bytes 0-3 | Bytes 4-15 |
+ |------------|---------------------------------------|
+ | length | data (padded with 0) |
+
+ * Long strings, length > 12
+ | Bytes 0-3 | Bytes 4-7 | Bytes 8-11 | Bytes 12-15 |
+ |------------|------------|------------|-------------|
+ | length | prefix | buf. index | offset |
+
+In both the long and short string cases, the first four bytes encode the
+length of the string and can be used to determine how the rest of the view
+should be interpreted.
+
+In the short string case the string's bytes are inlined- stored inside the
+view itself, in the twelve bytes which follow the length.
+
+In the long string case, a buffer index indicates which character buffer
+stores the characters and an offset indicates where in that buffer the
+characters begin. Buffer index 0 refers to the first character buffer, IE
+the first buffer **after** the validity buffer and the views buffer.
+The half-open range ``[offset, offset + length)`` must be entirely contained
+within the indicated buffer. A copy of the first four bytes of the string is
+stored inline in the prefix, after the length. This prefix enables a
+profitable fast path for string comparisons, which are frequently determined
+within the first four bytes.
+
+Views must be aligned to an 8-byte boundary. This restriction enables more
+efficient interoperation with systems where the index and offset are replaced
+by a raw pointer. All integers (length, buffer index, and offset) are unsigned
Review Comment:
When swizzling between index/offset and pointers, we frequently have sole ownership of the views buffer and can overwrite it directly. This isn't zero cost but it does save an allocation. This optimization isn't possible if an index/offset is not aligned to an 8 byte boundary since we can't overwrite them with a pointer in that case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323414076
##########
format/Message.fbs:
##########
@@ -99,6 +99,14 @@ table RecordBatch {
/// Optional compression of the message body
compression: BodyCompression;
+
+ /// Some types such as Utf8View are represented using a variable number of buffers.
+ /// For each such Field in the pre-ordered flattened logical schema, there will be
Review Comment:
Perhaps the IPC format addition should be covered somewhere in
https://arrow.apache.org/docs/format/Columnar.html#recordbatch-message?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] wjones127 commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "wjones127 (via GitHub)" <gi...@apache.org>.
wjones127 commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323574646
##########
docs/source/format/Columnar.rst:
##########
@@ -106,8 +106,10 @@ the different physical layouts defined by Arrow:
* **Primitive (fixed-size)**: a sequence of values each having the
same byte or bit width
* **Variable-size Binary**: a sequence of values each having a variable
- byte length. Two variants of this layout are supported using 32-bit
- and 64-bit length encoding.
+ byte length. Three variants of this layout are supported using
+ * 32-bit offset encoding
+ * 64-bit offset encoding
+ * 128-bit view-or-inline encoding
Review Comment:
As in: a separate bullet from `Variable-size Binary`? If so I'd agree, though if we do we should make sure to differentiate it by saying something like "Unlike the Variable-size binary layout, the elements in this layout may be out-of-order or even overlapping." Without any clarification on the differences, I agree it might be confusing for a reader of the spec.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323063664
##########
dev/archery/archery/integration/datagen.py:
##########
@@ -743,6 +763,82 @@ class LargeStringColumn(_BaseStringColumn, _LargeOffsetsMixin):
pass
+class BinaryViewColumn(PrimitiveColumn):
+
+ def _encode_value(self, x):
+ return frombytes(binascii.hexlify(x).upper())
+
+ def _get_buffers(self):
+ char_buffers = []
+ # a small default char buffer size is used so we get multiple
+ # character buffers without massive arrays
+ DEFAULT_BUFFER_SIZE = 32
+ INLINE_SIZE = 12
+
+ data = []
+ for i, v in enumerate(self.values):
+ if not self.is_valid[i]:
+ v = b''
+ assert isinstance(v, bytes)
+
+ if len(v) > INLINE_SIZE:
+ offset = 0
+ if len(v) > DEFAULT_BUFFER_SIZE:
+ # This string doesn't fit into a default sized char buffer;
+ # add it whole as a self-contained character buffer.
+ char_buffers.append(v)
+ elif len(char_buffers) == 0:
+ # No character buffers have been added yet;
+ # add this string whole (we may append to it later).
+ char_buffers.append(v)
+ elif len(char_buffers[-1]) + len(v) > DEFAULT_BUFFER_SIZE:
+ # Appending this string to the current active char buffer
+ # would overflow the default buffer size; add it whole.
+ char_buffers.append(v)
+ else:
+ # Append this string to the current active char buffer.
+ offset = len(char_buffers[-1])
+ char_buffers[-1] += v
+ # Sanity check that we haven't produced a char buffer
+ # longer than the default:
+ assert len(char_buffers[-1]) <= DEFAULT_BUFFER_SIZE
+
+ buffer_index = len(char_buffers) - 1
+
+ # the prefix is always 4 bytes so it may not be utf-8
+ # even if the whole string view is
+ prefix = v[:4].ljust(4, b'\0')
+ prefix = frombytes(binascii.hexlify(prefix).upper())
+
+ data.append(OrderedDict([
Review Comment:
Use of `OrderedDict` should not be necessary since Python 3.7, as dicts now follow insertion order.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1332298793
##########
docs/source/format/Columnar.rst:
##########
@@ -350,6 +352,53 @@ will be represented as follows: ::
|----------------|-----------------------|
| joemark | unspecified (padding) |
+.. versionadded:: Arrow Columnar Format 1.4
+
+Variable-size Binary View Layout
+--------------------------------
+
+Each value in this layout consists of 0 or more bytes. These characters'
+locations are indicated using a **views** buffer, which may point to one
+of potentially several **data** buffers or may contain the characters
+inline.
+
+The views buffer contains `length` view structures with the following layout:
+
+::
+
+ * Short strings, length <= 12
+ | Bytes 0-3 | Bytes 4-15 |
+ |------------|---------------------------------------|
+ | length | data (padded with 0) |
+
+ * Long strings, length > 12
+ | Bytes 0-3 | Bytes 4-7 | Bytes 8-11 | Bytes 12-15 |
+ |------------|------------|------------|-------------|
+ | length | prefix | buf. index | offset |
+
+In both the long and short string cases, the first four bytes encode the
+length of the string and can be used to determine how the rest of the view
+should be interpreted.
+
+In the short string case the string's bytes are inlined- stored inside the
+view itself, in the twelve bytes which follow the length.
+
+In the long string case, a buffer index indicates which character buffer
+stores the characters and an offset indicates where in that buffer the
+characters begin. Buffer index 0 refers to the first character buffer, IE
+the first buffer **after** the validity buffer and the views buffer.
+The half-open range ``[offset, offset + length)`` must be entirely contained
+within the indicated buffer. A copy of the first four bytes of the string is
+stored inline in the prefix, after the length. This prefix enables a
+profitable fast path for string comparisons, which are frequently determined
+within the first four bytes.
+
+Views must be aligned to an 8-byte boundary. This restriction enables more
+efficient interoperation with systems where the index and offset are replaced
+by a raw pointer. All integers (length, buffer index, and offset) are unsigned
Review Comment:
Per the mailing list, I've updated this to signed integers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] tustvold commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1317778621
##########
docs/source/format/Columnar.rst:
##########
@@ -350,6 +352,53 @@ will be represented as follows: ::
|----------------|-----------------------|
| joemark | unspecified (padding) |
+.. versionadded:: Arrow Columnar Format 1.4
+
+Variable-size Binary View Layout
+--------------------------------
+
+Each value in this layout consists of 0 or more bytes. These characters'
+locations are indicated using a **views** buffer, which may point to one
+of potentially several **data** buffers or may contain the characters
+inline.
+
+The views buffer contains `length` view structures with the following layout:
+
+::
+
+ * Short strings, length <= 12
+ | Bytes 0-3 | Bytes 4-15 |
+ |------------|---------------------------------------|
+ | length | data (padded with 0) |
+
+ * Long strings, length > 12
+ | Bytes 0-3 | Bytes 4-7 | Bytes 8-11 | Bytes 12-15 |
+ |------------|------------|------------|-------------|
+ | length | prefix | buf. index | offset |
+
+In both the long and short string cases, the first four bytes encode the
+length of the string and can be used to determine how the rest of the view
+should be interpreted.
+
+In the short string case the string's bytes are inlined- stored inside the
+view itself, in the twelve bytes which follow the length.
+
+In the long string case, a buffer index indicates which character buffer
+stores the characters and an offset indicates where in that buffer the
+characters begin. Buffer index 0 refers to the first character buffer, IE
+the first buffer **after** the validity buffer and the views buffer.
+The half-open range ``[offset, offset + length)`` must be entirely contained
+within the indicated buffer. A copy of the first four bytes of the string is
+stored inline in the prefix, after the length. This prefix enables a
+profitable fast path for string comparisons, which are frequently determined
+within the first four bytes.
+
+Views must be aligned to an 8-byte boundary. This restriction enables more
+efficient interoperation with systems where the index and offset are replaced
+by a raw pointer. All integers (length, buffer index, and offset) are unsigned
Review Comment:
I have to say I don't really understand this requirement, if the data is being mutated that necessarily implies a copy anyway?
##########
docs/source/format/Columnar.rst:
##########
@@ -350,6 +352,53 @@ will be represented as follows: ::
|----------------|-----------------------|
| joemark | unspecified (padding) |
+.. versionadded:: Arrow Columnar Format 1.4
+
+Variable-size Binary View Layout
+--------------------------------
+
+Each value in this layout consists of 0 or more bytes. These characters'
+locations are indicated using a **views** buffer, which may point to one
+of potentially several **data** buffers or may contain the characters
+inline.
+
+The views buffer contains `length` view structures with the following layout:
+
+::
+
+ * Short strings, length <= 12
+ | Bytes 0-3 | Bytes 4-15 |
+ |------------|---------------------------------------|
+ | length | data (padded with 0) |
+
+ * Long strings, length > 12
+ | Bytes 0-3 | Bytes 4-7 | Bytes 8-11 | Bytes 12-15 |
+ |------------|------------|------------|-------------|
+ | length | prefix | buf. index | offset |
+
+In both the long and short string cases, the first four bytes encode the
+length of the string and can be used to determine how the rest of the view
+should be interpreted.
+
+In the short string case the string's bytes are inlined- stored inside the
+view itself, in the twelve bytes which follow the length.
+
+In the long string case, a buffer index indicates which character buffer
+stores the characters and an offset indicates where in that buffer the
+characters begin. Buffer index 0 refers to the first character buffer, IE
+the first buffer **after** the validity buffer and the views buffer.
+The half-open range ``[offset, offset + length)`` must be entirely contained
+within the indicated buffer. A copy of the first four bytes of the string is
+stored inline in the prefix, after the length. This prefix enables a
+profitable fast path for string comparisons, which are frequently determined
+within the first four bytes.
+
+Views must be aligned to an 8-byte boundary. This restriction enables more
+efficient interoperation with systems where the index and offset are replaced
+by a raw pointer. All integers (length, buffer index, and offset) are unsigned
Review Comment:
I have to say I don't really understand this requirement, if the data is being mutated to convert to pointers that necessarily implies a copy anyway?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323040218
##########
docs/source/format/Columnar.rst:
##########
@@ -350,6 +352,53 @@ will be represented as follows: ::
|----------------|-----------------------|
| joemark | unspecified (padding) |
+.. versionadded:: Arrow Columnar Format 1.4
+
+Variable-size Binary View Layout
+--------------------------------
Review Comment:
Should move the `versionadded` below the title to avoid any confusion
```suggestion
Variable-size Binary View Layout
--------------------------------
.. versionadded:: Arrow Columnar Format 1.4
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323083898
##########
dev/archery/archery/integration/datagen.py:
##########
@@ -743,6 +763,82 @@ class LargeStringColumn(_BaseStringColumn, _LargeOffsetsMixin):
pass
+class BinaryViewColumn(PrimitiveColumn):
+
+ def _encode_value(self, x):
+ return frombytes(binascii.hexlify(x).upper())
+
+ def _get_buffers(self):
+ char_buffers = []
+ # a small default char buffer size is used so we get multiple
+ # character buffers without massive arrays
+ DEFAULT_BUFFER_SIZE = 32
Review Comment:
Ah, you're right.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323129678
##########
dev/archery/archery/integration/datagen.py:
##########
@@ -743,6 +763,82 @@ class LargeStringColumn(_BaseStringColumn, _LargeOffsetsMixin):
pass
+class BinaryViewColumn(PrimitiveColumn):
+
+ def _encode_value(self, x):
+ return frombytes(binascii.hexlify(x).upper())
+
+ def _get_buffers(self):
+ char_buffers = []
+ # a small default char buffer size is used so we get multiple
+ # character buffers without massive arrays
+ DEFAULT_BUFFER_SIZE = 32
+ INLINE_SIZE = 12
+
+ data = []
+ for i, v in enumerate(self.values):
+ if not self.is_valid[i]:
+ v = b''
+ assert isinstance(v, bytes)
+
+ if len(v) > INLINE_SIZE:
+ offset = 0
+ if len(v) > DEFAULT_BUFFER_SIZE:
+ # This string doesn't fit into a default sized char buffer;
+ # add it whole as a self-contained character buffer.
+ char_buffers.append(v)
+ elif len(char_buffers) == 0:
+ # No character buffers have been added yet;
+ # add this string whole (we may append to it later).
+ char_buffers.append(v)
+ elif len(char_buffers[-1]) + len(v) > DEFAULT_BUFFER_SIZE:
+ # Appending this string to the current active char buffer
+ # would overflow the default buffer size; add it whole.
+ char_buffers.append(v)
+ else:
+ # Append this string to the current active char buffer.
+ offset = len(char_buffers[-1])
+ char_buffers[-1] += v
+ # Sanity check that we haven't produced a char buffer
+ # longer than the default:
+ assert len(char_buffers[-1]) <= DEFAULT_BUFFER_SIZE
+
+ buffer_index = len(char_buffers) - 1
+
+ # the prefix is always 4 bytes so it may not be utf-8
+ # even if the whole string view is
+ prefix = v[:4].ljust(4, b'\0')
+ prefix = frombytes(binascii.hexlify(prefix).upper())
+
+ data.append(OrderedDict([
+ ('SIZE', len(v)),
+ ('PREFIX_HEX', prefix),
+ ('BUFFER_INDEX', buffer_index),
+ ('OFFSET', offset),
+ ]))
Review Comment:
I considered adding the variadic buffers as siblings of VALIDITY and DATA, but then we have VARIADIC_BUFFER_0, VARIADIC_BUFFER_1, ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323059686
##########
dev/archery/archery/integration/datagen.py:
##########
@@ -743,6 +763,82 @@ class LargeStringColumn(_BaseStringColumn, _LargeOffsetsMixin):
pass
+class BinaryViewColumn(PrimitiveColumn):
+
+ def _encode_value(self, x):
+ return frombytes(binascii.hexlify(x).upper())
+
+ def _get_buffers(self):
+ char_buffers = []
+ # a small default char buffer size is used so we get multiple
+ # character buffers without massive arrays
+ DEFAULT_BUFFER_SIZE = 32
+ INLINE_SIZE = 12
+
+ data = []
+ for i, v in enumerate(self.values):
+ if not self.is_valid[i]:
+ v = b''
+ assert isinstance(v, bytes)
+
+ if len(v) > INLINE_SIZE:
+ offset = 0
+ if len(v) > DEFAULT_BUFFER_SIZE:
+ # This string doesn't fit into a default sized char buffer;
+ # add it whole as a self-contained character buffer.
+ char_buffers.append(v)
+ elif len(char_buffers) == 0:
+ # No character buffers have been added yet;
+ # add this string whole (we may append to it later).
+ char_buffers.append(v)
+ elif len(char_buffers[-1]) + len(v) > DEFAULT_BUFFER_SIZE:
+ # Appending this string to the current active char buffer
+ # would overflow the default buffer size; add it whole.
+ char_buffers.append(v)
+ else:
+ # Append this string to the current active char buffer.
+ offset = len(char_buffers[-1])
+ char_buffers[-1] += v
+ # Sanity check that we haven't produced a char buffer
+ # longer than the default:
+ assert len(char_buffers[-1]) <= DEFAULT_BUFFER_SIZE
+
+ buffer_index = len(char_buffers) - 1
+
+ # the prefix is always 4 bytes so it may not be utf-8
+ # even if the whole string view is
+ prefix = v[:4].ljust(4, b'\0')
Review Comment:
Nit, but `ljust` isn't needed here as ` v` is always larger than the inline prefix size.
##########
dev/archery/archery/integration/datagen.py:
##########
@@ -743,6 +763,82 @@ class LargeStringColumn(_BaseStringColumn, _LargeOffsetsMixin):
pass
+class BinaryViewColumn(PrimitiveColumn):
+
+ def _encode_value(self, x):
+ return frombytes(binascii.hexlify(x).upper())
+
+ def _get_buffers(self):
+ char_buffers = []
+ # a small default char buffer size is used so we get multiple
+ # character buffers without massive arrays
+ DEFAULT_BUFFER_SIZE = 32
+ INLINE_SIZE = 12
+
+ data = []
+ for i, v in enumerate(self.values):
+ if not self.is_valid[i]:
+ v = b''
+ assert isinstance(v, bytes)
+
+ if len(v) > INLINE_SIZE:
+ offset = 0
+ if len(v) > DEFAULT_BUFFER_SIZE:
+ # This string doesn't fit into a default sized char buffer;
+ # add it whole as a self-contained character buffer.
+ char_buffers.append(v)
+ elif len(char_buffers) == 0:
+ # No character buffers have been added yet;
+ # add this string whole (we may append to it later).
+ char_buffers.append(v)
+ elif len(char_buffers[-1]) + len(v) > DEFAULT_BUFFER_SIZE:
+ # Appending this string to the current active char buffer
+ # would overflow the default buffer size; add it whole.
+ char_buffers.append(v)
+ else:
+ # Append this string to the current active char buffer.
+ offset = len(char_buffers[-1])
+ char_buffers[-1] += v
+ # Sanity check that we haven't produced a char buffer
+ # longer than the default:
+ assert len(char_buffers[-1]) <= DEFAULT_BUFFER_SIZE
+
+ buffer_index = len(char_buffers) - 1
+
+ # the prefix is always 4 bytes so it may not be utf-8
+ # even if the whole string view is
+ prefix = v[:4].ljust(4, b'\0')
Review Comment:
Nit, but `ljust` isn't needed here as ` v` is always larger than the inline prefix size.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323032277
##########
docs/source/format/Columnar.rst:
##########
@@ -880,19 +929,20 @@ For the avoidance of ambiguity, we provide listing the order and type
of memory buffers for each layout.
.. csv-table:: Buffer Layouts
- :header: "Layout Type", "Buffer 0", "Buffer 1", "Buffer 2"
- :widths: 30, 20, 20, 20
-
- "Primitive",validity,data,
- "Variable Binary",validity,offsets,data
- "List",validity,offsets,
- "Fixed-size List",validity,,
- "Struct",validity,,
- "Sparse Union",type ids,,
- "Dense Union",type ids,offsets,
- "Null",,,
- "Dictionary-encoded",validity,data (indices),
- "Run-end encoded",,,
+ :header: "Layout Type", "Buffer 0", "Buffer 1", "Buffer 2", "Variadic Buffers"
+ :widths: 30, 20, 20, 20, 20
+
+ "Primitive",validity,data,,
+ "Variable Binary",validity,offsets,data,
+ "Variable Binary - View",validity,packed headers,,data
Review Comment:
Nit, but I don't think the hyphen or dash is useful here?
Also, we use the term "view" rather than "headers" above and we don't mention packing.
```suggestion
"Variable Binary View",validity,views,,data
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323034338
##########
format/Schema.fbs:
##########
@@ -22,6 +22,7 @@
/// Version 1.1 - Add Decimal256.
/// Version 1.2 - Add Interval MONTH_DAY_NANO.
/// Version 1.3 - Add Run-End Encoded.
+/// Version 1.4 - Add BinaryView and Utf8View.
Review Comment:
```suggestion
/// Version 1.4 - Add BinaryView, Utf8View and variadicBufferCounts.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323147099
##########
docs/source/format/Columnar.rst:
##########
@@ -350,6 +352,53 @@ will be represented as follows: ::
|----------------|-----------------------|
| joemark | unspecified (padding) |
+.. versionadded:: Arrow Columnar Format 1.4
+
+Variable-size Binary View Layout
+--------------------------------
+
+Each value in this layout consists of 0 or more bytes. These characters'
+locations are indicated using a **views** buffer, which may point to one
+of potentially several **data** buffers or may contain the characters
+inline.
+
+The views buffer contains `length` view structures with the following layout:
+
+::
+
+ * Short strings, length <= 12
+ | Bytes 0-3 | Bytes 4-15 |
+ |------------|---------------------------------------|
+ | length | data (padded with 0) |
+
+ * Long strings, length > 12
+ | Bytes 0-3 | Bytes 4-7 | Bytes 8-11 | Bytes 12-15 |
+ |------------|------------|------------|-------------|
+ | length | prefix | buf. index | offset |
+
+In both the long and short string cases, the first four bytes encode the
+length of the string and can be used to determine how the rest of the view
+should be interpreted.
+
+In the short string case the string's bytes are inlined- stored inside the
+view itself, in the twelve bytes which follow the length.
+
+In the long string case, a buffer index indicates which character buffer
+stores the characters and an offset indicates where in that buffer the
+characters begin. Buffer index 0 refers to the first character buffer, IE
+the first buffer **after** the validity buffer and the views buffer.
+The half-open range ``[offset, offset + length)`` must be entirely contained
+within the indicated buffer. A copy of the first four bytes of the string is
+stored inline in the prefix, after the length. This prefix enables a
+profitable fast path for string comparisons, which are frequently determined
+within the first four bytes.
+
+Views must be aligned to an 8-byte boundary. This restriction enables more
+efficient interoperation with systems where the index and offset are replaced
+by a raw pointer. All integers (length, buffer index, and offset) are unsigned
Review Comment:
Alignment is especially useful in the string view case because for engines which need raw pointer views we can swizzle back and forth in place, so long as 64 bit alignment is maintained. It's true that this should not be phrased as a requirement. I'll rewrite this to be a more specific recommendation
See also: https://github.com/apache/arrow-rs/pull/4585#discussion_r1316442549
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323329576
##########
format/Message.fbs:
##########
@@ -99,6 +99,14 @@ table RecordBatch {
/// Optional compression of the message body
compression: BodyCompression;
+
+ /// Some types such as Utf8View are represented using a variable number of buffers.
+ /// For each such Field in the pre-ordered flattened logical schema, there will be
Review Comment:
It would be `[2, 1]` since it is only relevant to the subset of fields which carry a variable number of buffers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323057885
##########
dev/archery/archery/integration/datagen.py:
##########
@@ -743,6 +763,82 @@ class LargeStringColumn(_BaseStringColumn, _LargeOffsetsMixin):
pass
+class BinaryViewColumn(PrimitiveColumn):
+
+ def _encode_value(self, x):
+ return frombytes(binascii.hexlify(x).upper())
+
+ def _get_buffers(self):
+ char_buffers = []
+ # a small default char buffer size is used so we get multiple
+ # character buffers without massive arrays
+ DEFAULT_BUFFER_SIZE = 32
Review Comment:
Perhaps `MAX_DATA_BUFFER_SIZE`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1325056992
##########
docs/source/format/Columnar.rst:
##########
@@ -106,8 +106,10 @@ the different physical layouts defined by Arrow:
* **Primitive (fixed-size)**: a sequence of values each having the
same byte or bit width
* **Variable-size Binary**: a sequence of values each having a variable
- byte length. Two variants of this layout are supported using 32-bit
- and 64-bit length encoding.
+ byte length. Three variants of this layout are supported using
+ * 32-bit offset encoding
+ * 64-bit offset encoding
+ * 128-bit view-or-inline encoding
Review Comment:
I'll amend the list with a new bullet point for `Views of Variable-size Binary`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323141953
##########
dev/archery/archery/integration/datagen.py:
##########
@@ -743,6 +763,82 @@ class LargeStringColumn(_BaseStringColumn, _LargeOffsetsMixin):
pass
+class BinaryViewColumn(PrimitiveColumn):
+
+ def _encode_value(self, x):
+ return frombytes(binascii.hexlify(x).upper())
+
+ def _get_buffers(self):
+ char_buffers = []
+ # a small default char buffer size is used so we get multiple
+ # character buffers without massive arrays
+ DEFAULT_BUFFER_SIZE = 32
+ INLINE_SIZE = 12
+
+ data = []
+ for i, v in enumerate(self.values):
+ if not self.is_valid[i]:
+ v = b''
+ assert isinstance(v, bytes)
+
+ if len(v) > INLINE_SIZE:
+ offset = 0
+ if len(v) > DEFAULT_BUFFER_SIZE:
+ # This string doesn't fit into a default sized char buffer;
+ # add it whole as a self-contained character buffer.
+ char_buffers.append(v)
+ elif len(char_buffers) == 0:
+ # No character buffers have been added yet;
+ # add this string whole (we may append to it later).
+ char_buffers.append(v)
+ elif len(char_buffers[-1]) + len(v) > DEFAULT_BUFFER_SIZE:
+ # Appending this string to the current active char buffer
+ # would overflow the default buffer size; add it whole.
+ char_buffers.append(v)
+ else:
+ # Append this string to the current active char buffer.
+ offset = len(char_buffers[-1])
+ char_buffers[-1] += v
+ # Sanity check that we haven't produced a char buffer
+ # longer than the default:
+ assert len(char_buffers[-1]) <= DEFAULT_BUFFER_SIZE
+
+ buffer_index = len(char_buffers) - 1
+
+ # the prefix is always 4 bytes so it may not be utf-8
+ # even if the whole string view is
+ prefix = v[:4].ljust(4, b'\0')
+ prefix = frombytes(binascii.hexlify(prefix).upper())
+
+ data.append(OrderedDict([
+ ('SIZE', len(v)),
+ ('PREFIX_HEX', prefix),
+ ('BUFFER_INDEX', buffer_index),
+ ('OFFSET', offset),
+ ]))
Review Comment:
Oh, I see. This is the encoding of the *views* rather than of the variadic buffers. The views buffer is an array of structs in IPC, so I was copying that here. I'll rename the buffers `DATA -> VIEWS, VARIADIC_BUFFERS -> VARIADIC_DATA_BUFFERS` which I think would've avoided this confusion
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323232803
##########
docs/source/format/Columnar.rst:
##########
@@ -350,6 +352,57 @@ will be represented as follows: ::
|----------------|-----------------------|
| joemark | unspecified (padding) |
+Variable-size Binary View Layout
+--------------------------------
+
+.. versionadded:: Arrow Columnar Format 1.4
+
+Each value in this layout consists of 0 or more bytes. These bytes'
+locations are indicated using a **views** buffer, which may point to one
+of potentially several **data** buffers or may contain the characters
+inline.
+
+The views buffer contains `length` view structures with the following layout:
+
+::
+
+ * Short strings, length <= 12
+ | Bytes 0-3 | Bytes 4-15 |
+ |------------|---------------------------------------|
+ | length | data (padded with 0) |
+
+ * Long strings, length > 12
+ | Bytes 0-3 | Bytes 4-7 | Bytes 8-11 | Bytes 12-15 |
+ |------------|------------|------------|-------------|
+ | length | prefix | buf. index | offset |
+
+In both the long and short string cases, the first four bytes encode the
+length of the string and can be used to determine how the rest of the view
+should be interpreted.
+
+In the short string case the string's bytes are inlined- stored inside the
+view itself, in the twelve bytes which follow the length.
+
+In the long string case, a buffer index indicates which data buffer
+stores the data bytes and an offset indicates where in that buffer the
+data bytes begin. Buffer index 0 refers to the first data buffer, IE
+the first buffer **after** the validity buffer and the views buffer.
+The half-open range ``[offset, offset + length)`` must be entirely contained
+within the indicated buffer. A copy of the first four bytes of the string is
+stored inline in the prefix, after the length. This prefix enables a
+profitable fast path for string comparisons, which are frequently determined
+within the first four bytes.
+
+All integers (length, buffer index, and offset) are unsigned
+for compatibility with engines which already implement these views.
+
+This layout is adapted from TU Munich's `UmbraDB`_.
+
+It is worth noting that maintaining recommended `Buffer Alignment and Padding`_
Review Comment:
I don't think it is useful to keep this paragraph. It does not make it easier to understand or implement the Arrow format. Ease of conversion with other formats is not the purpose of this document.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] conbench-apache-arrow[bot] commented on pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #37526:
URL: https://github.com/apache/arrow/pull/37526#issuecomment-1730202195
After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 9d6d501fb654a93496070b59fbe7f36f4f1ed604.
There were 4 benchmark results indicating a performance regression:
- Commit Run on `ursa-i9-9960x` at [2023-09-21 12:33:35Z](https://conbench.ursa.dev/compare/runs/f83516b2eda14508b91200d2555c8600...12cb3f5ebd214645879db216d64795ed/)
- [`tpch` (R) with engine=arrow, format=native, language=R, memory_map=False, query_id=TPCH-11, scale_factor=1](https://conbench.ursa.dev/compare/benchmarks/0650bdf88da376678000b9dec5be4455...0650c5140cec711880004bab172faaf4)
- [`tpch` (R) with engine=arrow, format=parquet, language=R, memory_map=False, query_id=TPCH-10, scale_factor=1](https://conbench.ursa.dev/compare/benchmarks/0650bdf43e137f048000c3d16c4f43f0...0650c50e3f217f0780001a839e166065)
- and 2 more (see the report linked below)
The [full Conbench report](https://github.com/apache/arrow/runs/17017308792) has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323134232
##########
format/Schema.fbs:
##########
@@ -22,6 +22,7 @@
/// Version 1.1 - Add Decimal256.
/// Version 1.2 - Add Interval MONTH_DAY_NANO.
/// Version 1.3 - Add Run-End Encoded.
+/// Version 1.4 - Add BinaryView and Utf8View.
Review Comment:
```suggestion
/// Version 1.4 - Add BinaryView, Utf8View, and variadicBufferCounts.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323242081
##########
docs/source/format/Columnar.rst:
##########
@@ -350,6 +352,57 @@ will be represented as follows: ::
|----------------|-----------------------|
| joemark | unspecified (padding) |
+Variable-size Binary View Layout
+--------------------------------
+
+.. versionadded:: Arrow Columnar Format 1.4
+
+Each value in this layout consists of 0 or more bytes. These bytes'
+locations are indicated using a **views** buffer, which may point to one
+of potentially several **data** buffers or may contain the characters
+inline.
+
+The views buffer contains `length` view structures with the following layout:
+
+::
+
+ * Short strings, length <= 12
+ | Bytes 0-3 | Bytes 4-15 |
+ |------------|---------------------------------------|
+ | length | data (padded with 0) |
+
+ * Long strings, length > 12
+ | Bytes 0-3 | Bytes 4-7 | Bytes 8-11 | Bytes 12-15 |
+ |------------|------------|------------|-------------|
+ | length | prefix | buf. index | offset |
+
+In both the long and short string cases, the first four bytes encode the
+length of the string and can be used to determine how the rest of the view
+should be interpreted.
+
+In the short string case the string's bytes are inlined- stored inside the
+view itself, in the twelve bytes which follow the length.
+
+In the long string case, a buffer index indicates which data buffer
+stores the data bytes and an offset indicates where in that buffer the
+data bytes begin. Buffer index 0 refers to the first data buffer, IE
+the first buffer **after** the validity buffer and the views buffer.
+The half-open range ``[offset, offset + length)`` must be entirely contained
+within the indicated buffer. A copy of the first four bytes of the string is
+stored inline in the prefix, after the length. This prefix enables a
+profitable fast path for string comparisons, which are frequently determined
+within the first four bytes.
+
+All integers (length, buffer index, and offset) are unsigned
+for compatibility with engines which already implement these views.
+
+This layout is adapted from TU Munich's `UmbraDB`_.
+
+It is worth noting that maintaining recommended `Buffer Alignment and Padding`_
Review Comment:
In light of @tustvold's and your objections to the note, I'll remove it from Columnar.rst. It can be part of the doccomment for the swizzling function
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323080975
##########
dev/archery/archery/integration/datagen.py:
##########
@@ -743,6 +763,82 @@ class LargeStringColumn(_BaseStringColumn, _LargeOffsetsMixin):
pass
+class BinaryViewColumn(PrimitiveColumn):
+
+ def _encode_value(self, x):
+ return frombytes(binascii.hexlify(x).upper())
+
+ def _get_buffers(self):
+ char_buffers = []
+ # a small default char buffer size is used so we get multiple
+ # character buffers without massive arrays
+ DEFAULT_BUFFER_SIZE = 32
Review Comment:
But it's not the maximum; a string with more than 32 characters would provoke a buffer longer than 32 in order to store its data (alone)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323110020
##########
dev/archery/archery/integration/datagen.py:
##########
@@ -743,6 +763,82 @@ class LargeStringColumn(_BaseStringColumn, _LargeOffsetsMixin):
pass
+class BinaryViewColumn(PrimitiveColumn):
+
+ def _encode_value(self, x):
+ return frombytes(binascii.hexlify(x).upper())
+
+ def _get_buffers(self):
+ char_buffers = []
+ # a small default char buffer size is used so we get multiple
+ # character buffers without massive arrays
+ DEFAULT_BUFFER_SIZE = 32
+ INLINE_SIZE = 12
+
+ data = []
+ for i, v in enumerate(self.values):
+ if not self.is_valid[i]:
+ v = b''
+ assert isinstance(v, bytes)
+
+ if len(v) > INLINE_SIZE:
+ offset = 0
+ if len(v) > DEFAULT_BUFFER_SIZE:
+ # This string doesn't fit into a default sized char buffer;
+ # add it whole as a self-contained character buffer.
+ char_buffers.append(v)
+ elif len(char_buffers) == 0:
+ # No character buffers have been added yet;
+ # add this string whole (we may append to it later).
+ char_buffers.append(v)
+ elif len(char_buffers[-1]) + len(v) > DEFAULT_BUFFER_SIZE:
+ # Appending this string to the current active char buffer
+ # would overflow the default buffer size; add it whole.
+ char_buffers.append(v)
+ else:
+ # Append this string to the current active char buffer.
+ offset = len(char_buffers[-1])
+ char_buffers[-1] += v
+ # Sanity check that we haven't produced a char buffer
+ # longer than the default:
+ assert len(char_buffers[-1]) <= DEFAULT_BUFFER_SIZE
+
+ buffer_index = len(char_buffers) - 1
+
+ # the prefix is always 4 bytes so it may not be utf-8
+ # even if the whole string view is
+ prefix = v[:4].ljust(4, b'\0')
+ prefix = frombytes(binascii.hexlify(prefix).upper())
+
+ data.append(OrderedDict([
Review Comment:
I'd prefer to defer refactoring datagen.py away from OrderedDict. I'll open a follow up issue
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] alamb commented on pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #37526:
URL: https://github.com/apache/arrow/pull/37526#issuecomment-1730119869
🎉
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on PR #37526:
URL: https://github.com/apache/arrow/pull/37526#issuecomment-1729428033
+1, thanks all!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1325067239
##########
dev/archery/archery/integration/datagen.py:
##########
@@ -743,6 +763,82 @@ class LargeStringColumn(_BaseStringColumn, _LargeOffsetsMixin):
pass
+class BinaryViewColumn(PrimitiveColumn):
+
+ def _encode_value(self, x):
+ return frombytes(binascii.hexlify(x).upper())
+
+ def _get_buffers(self):
+ char_buffers = []
+ # a small default char buffer size is used so we get multiple
+ # character buffers without massive arrays
+ DEFAULT_BUFFER_SIZE = 32
+ INLINE_SIZE = 12
+
+ data = []
+ for i, v in enumerate(self.values):
+ if not self.is_valid[i]:
+ v = b''
+ assert isinstance(v, bytes)
+
+ if len(v) > INLINE_SIZE:
+ offset = 0
+ if len(v) > DEFAULT_BUFFER_SIZE:
+ # This string doesn't fit into a default sized char buffer;
+ # add it whole as a self-contained character buffer.
+ char_buffers.append(v)
+ elif len(char_buffers) == 0:
+ # No character buffers have been added yet;
+ # add this string whole (we may append to it later).
+ char_buffers.append(v)
+ elif len(char_buffers[-1]) + len(v) > DEFAULT_BUFFER_SIZE:
+ # Appending this string to the current active char buffer
+ # would overflow the default buffer size; add it whole.
+ char_buffers.append(v)
+ else:
+ # Append this string to the current active char buffer.
+ offset = len(char_buffers[-1])
+ char_buffers[-1] += v
+ # Sanity check that we haven't produced a char buffer
+ # longer than the default:
+ assert len(char_buffers[-1]) <= DEFAULT_BUFFER_SIZE
+
+ buffer_index = len(char_buffers) - 1
+
+ # the prefix is always 4 bytes so it may not be utf-8
+ # even if the whole string view is
+ prefix = v[:4].ljust(4, b'\0')
+ prefix = frombytes(binascii.hexlify(prefix).upper())
+
+ data.append(OrderedDict([
Review Comment:
https://github.com/apache/arrow/issues/37711
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] tustvold commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1318063179
##########
docs/source/format/Columnar.rst:
##########
@@ -350,6 +352,53 @@ will be represented as follows: ::
|----------------|-----------------------|
| joemark | unspecified (padding) |
+.. versionadded:: Arrow Columnar Format 1.4
+
+Variable-size Binary View Layout
+--------------------------------
+
+Each value in this layout consists of 0 or more bytes. These characters'
+locations are indicated using a **views** buffer, which may point to one
+of potentially several **data** buffers or may contain the characters
+inline.
+
+The views buffer contains `length` view structures with the following layout:
+
+::
+
+ * Short strings, length <= 12
+ | Bytes 0-3 | Bytes 4-15 |
+ |------------|---------------------------------------|
+ | length | data (padded with 0) |
+
+ * Long strings, length > 12
+ | Bytes 0-3 | Bytes 4-7 | Bytes 8-11 | Bytes 12-15 |
+ |------------|------------|------------|-------------|
+ | length | prefix | buf. index | offset |
+
+In both the long and short string cases, the first four bytes encode the
+length of the string and can be used to determine how the rest of the view
+should be interpreted.
+
+In the short string case the string's bytes are inlined- stored inside the
+view itself, in the twelve bytes which follow the length.
+
+In the long string case, a buffer index indicates which character buffer
+stores the characters and an offset indicates where in that buffer the
+characters begin. Buffer index 0 refers to the first character buffer, IE
+the first buffer **after** the validity buffer and the views buffer.
+The half-open range ``[offset, offset + length)`` must be entirely contained
+within the indicated buffer. A copy of the first four bytes of the string is
+stored inline in the prefix, after the length. This prefix enables a
+profitable fast path for string comparisons, which are frequently determined
+within the first four bytes.
+
+Views must be aligned to an 8-byte boundary. This restriction enables more
+efficient interoperation with systems where the index and offset are replaced
+by a raw pointer. All integers (length, buffer index, and offset) are unsigned
Review Comment:
Can't we just make the swizzling predicate - exclusive and aligned? It seems odd to mandate this and not for other types?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323016728
##########
docs/source/format/Columnar.rst:
##########
@@ -350,6 +352,53 @@ will be represented as follows: ::
|----------------|-----------------------|
| joemark | unspecified (padding) |
+.. versionadded:: Arrow Columnar Format 1.4
+
+Variable-size Binary View Layout
+--------------------------------
+
+Each value in this layout consists of 0 or more bytes. These characters'
Review Comment:
```suggestion
Each value in this layout consists of 0 or more bytes. These bytes'
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323015731
##########
docs/source/format/Columnar.rst:
##########
@@ -350,6 +352,53 @@ will be represented as follows: ::
|----------------|-----------------------|
| joemark | unspecified (padding) |
+.. versionadded:: Arrow Columnar Format 1.4
+
+Variable-size Binary View Layout
Review Comment:
... and indeed here this is described as a different layout.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323057517
##########
dev/archery/archery/integration/datagen.py:
##########
@@ -743,6 +763,82 @@ class LargeStringColumn(_BaseStringColumn, _LargeOffsetsMixin):
pass
+class BinaryViewColumn(PrimitiveColumn):
+
+ def _encode_value(self, x):
+ return frombytes(binascii.hexlify(x).upper())
+
+ def _get_buffers(self):
+ char_buffers = []
+ # a small default char buffer size is used so we get multiple
+ # character buffers without massive arrays
+ DEFAULT_BUFFER_SIZE = 32
+ INLINE_SIZE = 12
+
+ data = []
+ for i, v in enumerate(self.values):
+ if not self.is_valid[i]:
+ v = b''
+ assert isinstance(v, bytes)
+
+ if len(v) > INLINE_SIZE:
+ offset = 0
+ if len(v) > DEFAULT_BUFFER_SIZE:
Review Comment:
This is probably redundant with the third condition below (`len(char_buffers[-1]) + len(v) > DEFAULT_BUFFER_SIZE`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323130148
##########
dev/archery/archery/integration/datagen.py:
##########
@@ -743,6 +763,82 @@ class LargeStringColumn(_BaseStringColumn, _LargeOffsetsMixin):
pass
+class BinaryViewColumn(PrimitiveColumn):
+
+ def _encode_value(self, x):
+ return frombytes(binascii.hexlify(x).upper())
+
+ def _get_buffers(self):
+ char_buffers = []
+ # a small default char buffer size is used so we get multiple
+ # character buffers without massive arrays
+ DEFAULT_BUFFER_SIZE = 32
+ INLINE_SIZE = 12
+
+ data = []
+ for i, v in enumerate(self.values):
+ if not self.is_valid[i]:
+ v = b''
+ assert isinstance(v, bytes)
+
+ if len(v) > INLINE_SIZE:
+ offset = 0
+ if len(v) > DEFAULT_BUFFER_SIZE:
+ # This string doesn't fit into a default sized char buffer;
+ # add it whole as a self-contained character buffer.
+ char_buffers.append(v)
+ elif len(char_buffers) == 0:
+ # No character buffers have been added yet;
+ # add this string whole (we may append to it later).
+ char_buffers.append(v)
+ elif len(char_buffers[-1]) + len(v) > DEFAULT_BUFFER_SIZE:
+ # Appending this string to the current active char buffer
+ # would overflow the default buffer size; add it whole.
+ char_buffers.append(v)
+ else:
+ # Append this string to the current active char buffer.
+ offset = len(char_buffers[-1])
+ char_buffers[-1] += v
+ # Sanity check that we haven't produced a char buffer
+ # longer than the default:
+ assert len(char_buffers[-1]) <= DEFAULT_BUFFER_SIZE
+
+ buffer_index = len(char_buffers) - 1
+
+ # the prefix is always 4 bytes so it may not be utf-8
+ # even if the whole string view is
+ prefix = v[:4].ljust(4, b'\0')
+ prefix = frombytes(binascii.hexlify(prefix).upper())
+
+ data.append(OrderedDict([
+ ('SIZE', len(v)),
+ ('PREFIX_HEX', prefix),
+ ('BUFFER_INDEX', buffer_index),
+ ('OFFSET', offset),
+ ]))
Review Comment:
I'll add a comment
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] paleolimbot commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1330655138
##########
docs/source/format/Columnar.rst:
##########
@@ -350,6 +352,53 @@ will be represented as follows: ::
|----------------|-----------------------|
| joemark | unspecified (padding) |
+.. versionadded:: Arrow Columnar Format 1.4
+
+Variable-size Binary View Layout
+--------------------------------
+
+Each value in this layout consists of 0 or more bytes. These characters'
+locations are indicated using a **views** buffer, which may point to one
+of potentially several **data** buffers or may contain the characters
+inline.
+
+The views buffer contains `length` view structures with the following layout:
+
+::
+
+ * Short strings, length <= 12
+ | Bytes 0-3 | Bytes 4-15 |
+ |------------|---------------------------------------|
+ | length | data (padded with 0) |
+
+ * Long strings, length > 12
+ | Bytes 0-3 | Bytes 4-7 | Bytes 8-11 | Bytes 12-15 |
+ |------------|------------|------------|-------------|
+ | length | prefix | buf. index | offset |
+
+In both the long and short string cases, the first four bytes encode the
+length of the string and can be used to determine how the rest of the view
+should be interpreted.
+
+In the short string case the string's bytes are inlined- stored inside the
+view itself, in the twelve bytes which follow the length.
+
+In the long string case, a buffer index indicates which character buffer
+stores the characters and an offset indicates where in that buffer the
+characters begin. Buffer index 0 refers to the first character buffer, IE
+the first buffer **after** the validity buffer and the views buffer.
+The half-open range ``[offset, offset + length)`` must be entirely contained
+within the indicated buffer. A copy of the first four bytes of the string is
+stored inline in the prefix, after the length. This prefix enables a
+profitable fast path for string comparisons, which are frequently determined
+within the first four bytes.
+
+Views must be aligned to an 8-byte boundary. This restriction enables more
+efficient interoperation with systems where the index and offset are replaced
+by a raw pointer. All integers (length, buffer index, and offset) are unsigned
Review Comment:
I agree that the standard seems to have adopted signed integers for sizes in every other case. It seems very strange to impose that only for new types?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1330673342
##########
docs/source/format/Columnar.rst:
##########
@@ -350,6 +352,53 @@ will be represented as follows: ::
|----------------|-----------------------|
| joemark | unspecified (padding) |
+.. versionadded:: Arrow Columnar Format 1.4
+
+Variable-size Binary View Layout
+--------------------------------
+
+Each value in this layout consists of 0 or more bytes. These characters'
+locations are indicated using a **views** buffer, which may point to one
+of potentially several **data** buffers or may contain the characters
+inline.
+
+The views buffer contains `length` view structures with the following layout:
+
+::
+
+ * Short strings, length <= 12
+ | Bytes 0-3 | Bytes 4-15 |
+ |------------|---------------------------------------|
+ | length | data (padded with 0) |
+
+ * Long strings, length > 12
+ | Bytes 0-3 | Bytes 4-7 | Bytes 8-11 | Bytes 12-15 |
+ |------------|------------|------------|-------------|
+ | length | prefix | buf. index | offset |
+
+In both the long and short string cases, the first four bytes encode the
+length of the string and can be used to determine how the rest of the view
+should be interpreted.
+
+In the short string case the string's bytes are inlined- stored inside the
+view itself, in the twelve bytes which follow the length.
+
+In the long string case, a buffer index indicates which character buffer
+stores the characters and an offset indicates where in that buffer the
+characters begin. Buffer index 0 refers to the first character buffer, IE
+the first buffer **after** the validity buffer and the views buffer.
+The half-open range ``[offset, offset + length)`` must be entirely contained
+within the indicated buffer. A copy of the first four bytes of the string is
+stored inline in the prefix, after the length. This prefix enables a
+profitable fast path for string comparisons, which are frequently determined
+within the first four bytes.
+
+Views must be aligned to an 8-byte boundary. This restriction enables more
+efficient interoperation with systems where the index and offset are replaced
+by a raw pointer. All integers (length, buffer index, and offset) are unsigned
Review Comment:
It's not a requirement to be imposed on sizes in all cases going forward. It's only relevant to Utf8View because existing implementations of string view already use unsigned integers here, so the concern is whether to keep compatibility with those or maintain the convention of the arrow format. If you'd like to +1 signed integers, please say so on the ML!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323086797
##########
dev/archery/archery/integration/datagen.py:
##########
@@ -743,6 +763,82 @@ class LargeStringColumn(_BaseStringColumn, _LargeOffsetsMixin):
pass
+class BinaryViewColumn(PrimitiveColumn):
+
+ def _encode_value(self, x):
+ return frombytes(binascii.hexlify(x).upper())
+
+ def _get_buffers(self):
+ char_buffers = []
+ # a small default char buffer size is used so we get multiple
+ # character buffers without massive arrays
+ DEFAULT_BUFFER_SIZE = 32
+ INLINE_SIZE = 12
+
+ data = []
+ for i, v in enumerate(self.values):
+ if not self.is_valid[i]:
+ v = b''
+ assert isinstance(v, bytes)
+
+ if len(v) > INLINE_SIZE:
+ offset = 0
+ if len(v) > DEFAULT_BUFFER_SIZE:
Review Comment:
I'll try to reorganize
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323043512
##########
format/Schema.fbs:
##########
@@ -171,6 +172,24 @@ table LargeUtf8 {
table LargeBinary {
}
+/// Same as Utf8, but string characters are delimited with a packed
+/// length/pointer instead of offsets.
+
+/// Logically the same as Utf8, but the internal representation uses a header
Review Comment:
Nit: terminology
```suggestion
/// Logically the same as Utf8, but the internal representation uses a view
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323132879
##########
dev/archery/archery/integration/datagen.py:
##########
@@ -743,6 +763,82 @@ class LargeStringColumn(_BaseStringColumn, _LargeOffsetsMixin):
pass
+class BinaryViewColumn(PrimitiveColumn):
+
+ def _encode_value(self, x):
+ return frombytes(binascii.hexlify(x).upper())
+
+ def _get_buffers(self):
+ char_buffers = []
+ # a small default char buffer size is used so we get multiple
+ # character buffers without massive arrays
+ DEFAULT_BUFFER_SIZE = 32
+ INLINE_SIZE = 12
+
+ data = []
+ for i, v in enumerate(self.values):
+ if not self.is_valid[i]:
+ v = b''
+ assert isinstance(v, bytes)
+
+ if len(v) > INLINE_SIZE:
+ offset = 0
+ if len(v) > DEFAULT_BUFFER_SIZE:
+ # This string doesn't fit into a default sized char buffer;
+ # add it whole as a self-contained character buffer.
+ char_buffers.append(v)
+ elif len(char_buffers) == 0:
+ # No character buffers have been added yet;
+ # add this string whole (we may append to it later).
+ char_buffers.append(v)
+ elif len(char_buffers[-1]) + len(v) > DEFAULT_BUFFER_SIZE:
+ # Appending this string to the current active char buffer
+ # would overflow the default buffer size; add it whole.
+ char_buffers.append(v)
+ else:
+ # Append this string to the current active char buffer.
+ offset = len(char_buffers[-1])
+ char_buffers[-1] += v
+ # Sanity check that we haven't produced a char buffer
+ # longer than the default:
+ assert len(char_buffers[-1]) <= DEFAULT_BUFFER_SIZE
+
+ buffer_index = len(char_buffers) - 1
+
+ # the prefix is always 4 bytes so it may not be utf-8
+ # even if the whole string view is
+ prefix = v[:4].ljust(4, b'\0')
+ prefix = frombytes(binascii.hexlify(prefix).upper())
+
+ data.append(OrderedDict([
+ ('SIZE', len(v)),
+ ('PREFIX_HEX', prefix),
+ ('BUFFER_INDEX', buffer_index),
+ ('OFFSET', offset),
+ ]))
Review Comment:
SoA would rather be PACKED_SIZE_CHUNKS, PACKED_PREFIX_HEX_CHUNKS, etc. A bit less intuitive to read and generate.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323211280
##########
docs/source/format/Columnar.rst:
##########
@@ -350,6 +352,53 @@ will be represented as follows: ::
|----------------|-----------------------|
| joemark | unspecified (padding) |
+.. versionadded:: Arrow Columnar Format 1.4
+
+Variable-size Binary View Layout
+--------------------------------
+
+Each value in this layout consists of 0 or more bytes. These characters'
+locations are indicated using a **views** buffer, which may point to one
+of potentially several **data** buffers or may contain the characters
+inline.
+
+The views buffer contains `length` view structures with the following layout:
+
+::
+
+ * Short strings, length <= 12
+ | Bytes 0-3 | Bytes 4-15 |
+ |------------|---------------------------------------|
+ | length | data (padded with 0) |
+
+ * Long strings, length > 12
+ | Bytes 0-3 | Bytes 4-7 | Bytes 8-11 | Bytes 12-15 |
+ |------------|------------|------------|-------------|
+ | length | prefix | buf. index | offset |
+
+In both the long and short string cases, the first four bytes encode the
+length of the string and can be used to determine how the rest of the view
+should be interpreted.
+
+In the short string case the string's bytes are inlined- stored inside the
+view itself, in the twelve bytes which follow the length.
+
+In the long string case, a buffer index indicates which character buffer
+stores the characters and an offset indicates where in that buffer the
+characters begin. Buffer index 0 refers to the first character buffer, IE
+the first buffer **after** the validity buffer and the views buffer.
+The half-open range ``[offset, offset + length)`` must be entirely contained
+within the indicated buffer. A copy of the first four bytes of the string is
+stored inline in the prefix, after the length. This prefix enables a
+profitable fast path for string comparisons, which are frequently determined
+within the first four bytes.
+
+Views must be aligned to an 8-byte boundary. This restriction enables more
+efficient interoperation with systems where the index and offset are replaced
+by a raw pointer. All integers (length, buffer index, and offset) are unsigned
Review Comment:
I don't understand what "swizzle" means in this context, but I still don't understand how this is different from, say, MONTH_DAY_NANO_INTERVAL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz merged pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz merged PR #37526:
URL: https://github.com/apache/arrow/pull/37526
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323029022
##########
docs/source/format/Columnar.rst:
##########
@@ -350,6 +352,53 @@ will be represented as follows: ::
|----------------|-----------------------|
| joemark | unspecified (padding) |
+.. versionadded:: Arrow Columnar Format 1.4
+
+Variable-size Binary View Layout
+--------------------------------
+
+Each value in this layout consists of 0 or more bytes. These characters'
+locations are indicated using a **views** buffer, which may point to one
+of potentially several **data** buffers or may contain the characters
+inline.
+
+The views buffer contains `length` view structures with the following layout:
+
+::
+
+ * Short strings, length <= 12
+ | Bytes 0-3 | Bytes 4-15 |
+ |------------|---------------------------------------|
+ | length | data (padded with 0) |
+
+ * Long strings, length > 12
+ | Bytes 0-3 | Bytes 4-7 | Bytes 8-11 | Bytes 12-15 |
+ |------------|------------|------------|-------------|
+ | length | prefix | buf. index | offset |
+
+In both the long and short string cases, the first four bytes encode the
+length of the string and can be used to determine how the rest of the view
+should be interpreted.
+
+In the short string case the string's bytes are inlined- stored inside the
+view itself, in the twelve bytes which follow the length.
+
+In the long string case, a buffer index indicates which character buffer
+stores the characters and an offset indicates where in that buffer the
+characters begin. Buffer index 0 refers to the first character buffer, IE
+the first buffer **after** the validity buffer and the views buffer.
+The half-open range ``[offset, offset + length)`` must be entirely contained
+within the indicated buffer. A copy of the first four bytes of the string is
+stored inline in the prefix, after the length. This prefix enables a
+profitable fast path for string comparisons, which are frequently determined
+within the first four bytes.
+
+Views must be aligned to an 8-byte boundary. This restriction enables more
+efficient interoperation with systems where the index and offset are replaced
+by a raw pointer. All integers (length, buffer index, and offset) are unsigned
Review Comment:
Please let's please not make a special case for binary-view here. We use signed offsets and lengths everywhere else.
```suggestion
by a raw pointer. All integers (length, buffer index, and offset) are signed
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323021085
##########
docs/source/format/Columnar.rst:
##########
@@ -350,6 +352,53 @@ will be represented as follows: ::
|----------------|-----------------------|
| joemark | unspecified (padding) |
+.. versionadded:: Arrow Columnar Format 1.4
+
+Variable-size Binary View Layout
+--------------------------------
+
+Each value in this layout consists of 0 or more bytes. These characters'
+locations are indicated using a **views** buffer, which may point to one
+of potentially several **data** buffers or may contain the characters
+inline.
+
+The views buffer contains `length` view structures with the following layout:
+
+::
+
+ * Short strings, length <= 12
+ | Bytes 0-3 | Bytes 4-15 |
+ |------------|---------------------------------------|
+ | length | data (padded with 0) |
+
+ * Long strings, length > 12
+ | Bytes 0-3 | Bytes 4-7 | Bytes 8-11 | Bytes 12-15 |
+ |------------|------------|------------|-------------|
+ | length | prefix | buf. index | offset |
+
+In both the long and short string cases, the first four bytes encode the
+length of the string and can be used to determine how the rest of the view
+should be interpreted.
+
+In the short string case the string's bytes are inlined- stored inside the
+view itself, in the twelve bytes which follow the length.
+
+In the long string case, a buffer index indicates which character buffer
+stores the characters and an offset indicates where in that buffer the
+characters begin. Buffer index 0 refers to the first character buffer, IE
Review Comment:
```suggestion
In the long string case, a buffer index indicates which data buffer
stores the data bytes and an offset indicates where in that buffer the
data bytes begin. Buffer index 0 refers to the first data buffer, IE
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] mapleFU commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323311805
##########
format/Message.fbs:
##########
@@ -99,6 +99,14 @@ table RecordBatch {
/// Optional compression of the message body
compression: BodyCompression;
+
+ /// Some types such as Utf8View are represented using a variable number of buffers.
+ /// For each such Field in the pre-ordered flattened logical schema, there will be
Review Comment:
Just want to make sure I'm understand, for:
```
struct<i:int32, bv:BinaryView, uv: Utf8View>
```
If bv has 2 "Variadic Buffer"s, uv has 1 "Variadic Buffer", how would be `variadicBufferCounts` be? `[2, 1]` or `[0, 2, 1]`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323066393
##########
dev/archery/archery/integration/datagen.py:
##########
@@ -743,6 +763,82 @@ class LargeStringColumn(_BaseStringColumn, _LargeOffsetsMixin):
pass
+class BinaryViewColumn(PrimitiveColumn):
+
+ def _encode_value(self, x):
+ return frombytes(binascii.hexlify(x).upper())
+
+ def _get_buffers(self):
+ char_buffers = []
+ # a small default char buffer size is used so we get multiple
+ # character buffers without massive arrays
+ DEFAULT_BUFFER_SIZE = 32
+ INLINE_SIZE = 12
+
+ data = []
+ for i, v in enumerate(self.values):
+ if not self.is_valid[i]:
+ v = b''
+ assert isinstance(v, bytes)
+
+ if len(v) > INLINE_SIZE:
+ offset = 0
+ if len(v) > DEFAULT_BUFFER_SIZE:
+ # This string doesn't fit into a default sized char buffer;
+ # add it whole as a self-contained character buffer.
+ char_buffers.append(v)
+ elif len(char_buffers) == 0:
+ # No character buffers have been added yet;
+ # add this string whole (we may append to it later).
+ char_buffers.append(v)
+ elif len(char_buffers[-1]) + len(v) > DEFAULT_BUFFER_SIZE:
+ # Appending this string to the current active char buffer
+ # would overflow the default buffer size; add it whole.
+ char_buffers.append(v)
+ else:
+ # Append this string to the current active char buffer.
+ offset = len(char_buffers[-1])
+ char_buffers[-1] += v
+ # Sanity check that we haven't produced a char buffer
+ # longer than the default:
+ assert len(char_buffers[-1]) <= DEFAULT_BUFFER_SIZE
+
+ buffer_index = len(char_buffers) - 1
+
+ # the prefix is always 4 bytes so it may not be utf-8
+ # even if the whole string view is
+ prefix = v[:4].ljust(4, b'\0')
+ prefix = frombytes(binascii.hexlify(prefix).upper())
+
+ data.append(OrderedDict([
+ ('SIZE', len(v)),
+ ('PREFIX_HEX', prefix),
+ ('BUFFER_INDEX', buffer_index),
+ ('OFFSET', offset),
+ ]))
Review Comment:
So, FTR, this means the JSON encoding of the variadic buffers uses AoS instead of SoA for other types and buffers. Perhaps add a comment to note this. This should not be a problem in practice unless we generate non-small data.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323634991
##########
format/Message.fbs:
##########
@@ -99,6 +99,14 @@ table RecordBatch {
/// Optional compression of the message body
compression: BodyCompression;
+
+ /// Some types such as Utf8View are represented using a variable number of buffers.
+ /// For each such Field in the pre-ordered flattened logical schema, there will be
Review Comment:
I've added a section describing variadicBufferCounts with an example of explicit flattening
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323037564
##########
format/Message.fbs:
##########
@@ -99,6 +99,12 @@ table RecordBatch {
/// Optional compression of the message body
compression: BodyCompression;
+
+ /// Some types such as Utf8View are represented using a variable number of buffers.
+ /// For each such Field in the pre-ordered flattened logical schema, there will be
+ /// an entry in variadicBufferCounts to indicate the number of extra buffers which
+ /// belong to that Field.
Review Comment:
Not sure it's worth spelling it out explicitly, but suggestion below:
```suggestion
/// belong to that Field.
/// If this is omitted, then the data must not contain any Field with a variable
/// number of buffers, such as BinaryView and Utf8View.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323042405
##########
format/Schema.fbs:
##########
@@ -171,6 +172,24 @@ table LargeUtf8 {
table LargeBinary {
}
+/// Same as Utf8, but string characters are delimited with a packed
+/// length/pointer instead of offsets.
+
Review Comment:
Not sure this is useful as the description just below is more precise.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323028095
##########
docs/source/format/Columnar.rst:
##########
@@ -350,6 +352,53 @@ will be represented as follows: ::
|----------------|-----------------------|
| joemark | unspecified (padding) |
+.. versionadded:: Arrow Columnar Format 1.4
+
+Variable-size Binary View Layout
+--------------------------------
+
+Each value in this layout consists of 0 or more bytes. These characters'
+locations are indicated using a **views** buffer, which may point to one
+of potentially several **data** buffers or may contain the characters
+inline.
+
+The views buffer contains `length` view structures with the following layout:
+
+::
+
+ * Short strings, length <= 12
+ | Bytes 0-3 | Bytes 4-15 |
+ |------------|---------------------------------------|
+ | length | data (padded with 0) |
+
+ * Long strings, length > 12
+ | Bytes 0-3 | Bytes 4-7 | Bytes 8-11 | Bytes 12-15 |
+ |------------|------------|------------|-------------|
+ | length | prefix | buf. index | offset |
+
+In both the long and short string cases, the first four bytes encode the
+length of the string and can be used to determine how the rest of the view
+should be interpreted.
+
+In the short string case the string's bytes are inlined- stored inside the
+view itself, in the twelve bytes which follow the length.
+
+In the long string case, a buffer index indicates which character buffer
+stores the characters and an offset indicates where in that buffer the
+characters begin. Buffer index 0 refers to the first character buffer, IE
+the first buffer **after** the validity buffer and the views buffer.
+The half-open range ``[offset, offset + length)`` must be entirely contained
+within the indicated buffer. A copy of the first four bytes of the string is
+stored inline in the prefix, after the length. This prefix enables a
+profitable fast path for string comparisons, which are frequently determined
+within the first four bytes.
+
+Views must be aligned to an 8-byte boundary. This restriction enables more
+efficient interoperation with systems where the index and offset are replaced
+by a raw pointer. All integers (length, buffer index, and offset) are unsigned
Review Comment:
1. Other types don't have a specific alignment requirement even where it might be beneficial. Instead there is a general "Buffer Alignment and Padding" section. I don't think binary views are not special enough to warrant this.
2. I don't know of any CPU feature that would make alignment of pointer data more imperative than, say, alignment of 64-bit data.
3. The spec certainly shouldn't appear to condone or promote non-Arrow formats disguised as Arrow ("systems where the index and offset are replaced by a raw pointer").
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323233962
##########
docs/source/format/Columnar.rst:
##########
@@ -350,6 +352,57 @@ will be represented as follows: ::
|----------------|-----------------------|
| joemark | unspecified (padding) |
+Variable-size Binary View Layout
+--------------------------------
+
+.. versionadded:: Arrow Columnar Format 1.4
+
+Each value in this layout consists of 0 or more bytes. These bytes'
+locations are indicated using a **views** buffer, which may point to one
+of potentially several **data** buffers or may contain the characters
+inline.
+
+The views buffer contains `length` view structures with the following layout:
+
+::
+
+ * Short strings, length <= 12
+ | Bytes 0-3 | Bytes 4-15 |
+ |------------|---------------------------------------|
+ | length | data (padded with 0) |
+
+ * Long strings, length > 12
+ | Bytes 0-3 | Bytes 4-7 | Bytes 8-11 | Bytes 12-15 |
+ |------------|------------|------------|-------------|
+ | length | prefix | buf. index | offset |
+
+In both the long and short string cases, the first four bytes encode the
+length of the string and can be used to determine how the rest of the view
+should be interpreted.
+
+In the short string case the string's bytes are inlined- stored inside the
+view itself, in the twelve bytes which follow the length.
+
+In the long string case, a buffer index indicates which data buffer
+stores the data bytes and an offset indicates where in that buffer the
+data bytes begin. Buffer index 0 refers to the first data buffer, IE
+the first buffer **after** the validity buffer and the views buffer.
+The half-open range ``[offset, offset + length)`` must be entirely contained
+within the indicated buffer. A copy of the first four bytes of the string is
+stored inline in the prefix, after the length. This prefix enables a
+profitable fast path for string comparisons, which are frequently determined
+within the first four bytes.
+
+All integers (length, buffer index, and offset) are unsigned
+for compatibility with engines which already implement these views.
+
+This layout is adapted from TU Munich's `UmbraDB`_.
+
+It is worth noting that maintaining recommended `Buffer Alignment and Padding`_
Review Comment:
However, you may perhaps state that the recommended alignment is 16 bytes (or is it 8?) for ease of implementation reasons.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323047844
##########
format/Schema.fbs:
##########
@@ -171,6 +172,24 @@ table LargeUtf8 {
table LargeBinary {
}
+/// Same as Utf8, but string characters are delimited with a packed
+/// length/pointer instead of offsets.
+
+/// Logically the same as Utf8, but the internal representation uses a header
+/// struct that contains the string length and either the string's entire data
+/// inline (for small strings) or an inlined prefix, an index of another buffer,
+/// and an offset pointing to a slice in that buffer (for non-small strings).
Review Comment:
```suggestion
/// and an offset pointing to a slice in that buffer (for non-small strings).
///
/// Since it uses a variable number of data buffers, each Field with this type
/// must have a corresponding entry in `variadicBufferCounts`.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323067505
##########
dev/archery/archery/integration/datagen.py:
##########
@@ -1541,6 +1637,15 @@ def generate_run_end_encoded_case():
return _generate_file("run_end_encoded", fields, batch_sizes)
+def generate_view_case():
Review Comment:
Perhaps `generate_binary_view_case` since we'll probably make a separate case for list view.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323014928
##########
docs/source/format/Columnar.rst:
##########
@@ -106,8 +106,10 @@ the different physical layouts defined by Arrow:
* **Primitive (fixed-size)**: a sequence of values each having the
same byte or bit width
* **Variable-size Binary**: a sequence of values each having a variable
- byte length. Two variants of this layout are supported using 32-bit
- and 64-bit length encoding.
+ byte length. Three variants of this layout are supported using
+ * 32-bit offset encoding
+ * 64-bit offset encoding
+ * 128-bit view-or-inline encoding
Review Comment:
I think this is different enough to make this a different physical layout in this enumeration. WDYT @westonpace @wjones127 @alamb @wesm ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #37526: GH-35627: [Format][Integration] Add string-view to arrow format
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #37526:
URL: https://github.com/apache/arrow/pull/37526#discussion_r1323111795
##########
docs/source/format/Columnar.rst:
##########
@@ -350,6 +352,53 @@ will be represented as follows: ::
|----------------|-----------------------|
| joemark | unspecified (padding) |
+.. versionadded:: Arrow Columnar Format 1.4
+
+Variable-size Binary View Layout
+--------------------------------
+
+Each value in this layout consists of 0 or more bytes. These characters'
+locations are indicated using a **views** buffer, which may point to one
+of potentially several **data** buffers or may contain the characters
+inline.
+
+The views buffer contains `length` view structures with the following layout:
+
+::
+
+ * Short strings, length <= 12
+ | Bytes 0-3 | Bytes 4-15 |
+ |------------|---------------------------------------|
+ | length | data (padded with 0) |
+
+ * Long strings, length > 12
+ | Bytes 0-3 | Bytes 4-7 | Bytes 8-11 | Bytes 12-15 |
+ |------------|------------|------------|-------------|
+ | length | prefix | buf. index | offset |
+
+In both the long and short string cases, the first four bytes encode the
+length of the string and can be used to determine how the rest of the view
+should be interpreted.
+
+In the short string case the string's bytes are inlined- stored inside the
+view itself, in the twelve bytes which follow the length.
+
+In the long string case, a buffer index indicates which character buffer
+stores the characters and an offset indicates where in that buffer the
+characters begin. Buffer index 0 refers to the first character buffer, IE
+the first buffer **after** the validity buffer and the views buffer.
+The half-open range ``[offset, offset + length)`` must be entirely contained
+within the indicated buffer. A copy of the first four bytes of the string is
+stored inline in the prefix, after the length. This prefix enables a
+profitable fast path for string comparisons, which are frequently determined
+within the first four bytes.
+
+Views must be aligned to an 8-byte boundary. This restriction enables more
+efficient interoperation with systems where the index and offset are replaced
+by a raw pointer. All integers (length, buffer index, and offset) are unsigned
Review Comment:
We should discuss this on the ML, I'll raise this there
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org