You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@parquet.apache.org by "gh-yzou (via GitHub)" <gi...@apache.org> on 2025/05/28 18:18:50 UTC
[PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
gh-yzou opened a new pull request, #496:
URL: https://github.com/apache/parquet-format/pull/496
Add spec and type description for INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME 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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2155716617
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
Review Comment:
is the whole range of months valid or should it be limited to SQL standard (IIRC I think +/- 10000 years is what is generally called for)?
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2159816890
##########
LogicalTypes.md:
##########
@@ -539,6 +544,26 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### INTERVAL_YEAR_MONTH
+
+`INTERVAL_YEAR_MONTH` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### INTERVAL_DAY_TIME
+
+`INTERVAL_DAY_TIME` is used to represent a day-time time interval, such as
+`5 days, 10 hours and 30 minutes`. It must annotate a 16-byte `FIXED_LEN_BYTE_ARRAY`
+that stores the total number of nanoseconds representing the interval. The value is
+a signed integer and can be negative to indicate backward duration.
+The time interval is independent of any timezone.
+
+Based on the [ANSI SQL standard](https://docs.oracle.com/en/database/oracle/oracle-database/19/sqlrf/Data-Types.html#GUID-7690645A-0EE3-46CA-90DE-C96DF5A01F8F) definition of the INTERVAL data type and fields values,
+an interval of 1 day is equivalent to 24 hours, regardless of the specific number of
Review Comment:
i think the DayTimeInterval sometimes are referred as duration to be more clear on the unit used, and the pandas TimeDelta can be mapped to DayTimeInterval type, which is one of the biggest use case also. The YearMonth is clearly different as duration as @lidavidm pointed out.
@lidavidm
```
this detail should also probably be in parquet.thrift (reference to SQL standard probably isn't strictly necessary there, just how 1 day is defined).
```
is your suggestion to remove the SQL reference here?
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2160254815
##########
LogicalTypes.md:
##########
@@ -539,6 +544,26 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### INTERVAL_YEAR_MONTH
+
+`INTERVAL_YEAR_MONTH` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### INTERVAL_DAY_TIME
+
+`INTERVAL_DAY_TIME` is used to represent a day-time time interval, such as
+`5 days, 10 hours and 30 minutes`. It must annotate a 16-byte `FIXED_LEN_BYTE_ARRAY`
+that stores the total number of nanoseconds representing the interval. The value is
Review Comment:
Why call it "day-time" if it only stores nanoseconds? Please let's make the naming clear and non-misleading.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2164346973
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
Review Comment:
One other question here, is 16 bytes necessary, or should there be some smaller number of bytes (e.g. 10 or 11), or was 16 chosen for some zero copy purpose?
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2113019861
##########
LogicalTypes.md:
##########
@@ -539,6 +543,19 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### INTERVAL_YEAR_MONTH
+`INTERVAL_YEAR_MONTH` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### INTERVAL_DAY_TIME
+`INTERVAL_DAY_TIME` is used to represent a day-time time interval, such as
+`5 days, 10 hours and 30 minutes`. It must annotate and 16-byte `FIXED_LEN_BYTE_ARRAY`
Review Comment:
sg!, I am keeping that comment here, so we can carry the discussion along
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add YEAR_MONTH_INTERVAL and DURATION types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2231673045
##########
LogicalTypes.md:
##########
@@ -539,6 +544,27 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### YEAR_MONTH_INTERVAL
+
+`YEAR_MONTH_INTERVAL` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### DURATION
+
+`DURATION` is used to represent a span of time, such as `5 days`. It must
+annotate an `int64` value that stores the total number of time units for the
+duration. The value is a signed integer and can be negative to indicate backward duration.
+The duration is purely a measure of time and is independent of any time zone.
+
+The `DURATION` type takes `unit` as a parameter, and the value must be one of
+`MILLIS`, `MICROS` or `NANOS`.
+
+Duration of 1 day is defined exactly of 24 hours, regardless of the specific number of
Review Comment:
nit: I suggest moving this to be in the same a paragraph as `The duration is purely a measure of time and is independent of any time zone.` (split this paragraph off from the first paragraph).
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add YEAR_MONTH_INTERVAL and DURATION types [parquet-format]
Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2234692508
##########
LogicalTypes.md:
##########
@@ -539,6 +544,27 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### YEAR_MONTH_INTERVAL
+
+`YEAR_MONTH_INTERVAL` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### DURATION
+
+`DURATION` is used to represent a span of time, such as `5 days`. It must
+annotate an `int64` value that stores the total number of time units for the
+duration. The value is a signed integer and can be negative to indicate backward duration.
+The duration is purely a measure of time and is independent of any time zone.
+
+The `DURATION` type takes `unit` as a parameter, and the value must be one of
+`MILLIS`, `MICROS` or `NANOS`.
+
+Duration of 1 day is defined exactly of 24 hours, regardless of the specific number of
Review Comment:
make sense. mentioned ANSI SQL standard in the description
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,30 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct YearMonthIntervalType {
+}
+
+/**
+ * Day-Time Interval logical type annotation
Review Comment:
updated
##########
LogicalTypes.md:
##########
@@ -539,6 +544,27 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### YEAR_MONTH_INTERVAL
+
+`YEAR_MONTH_INTERVAL` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
Review Comment:
added description about the restriction.
##########
LogicalTypes.md:
##########
@@ -539,6 +544,27 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### YEAR_MONTH_INTERVAL
+
+`YEAR_MONTH_INTERVAL` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
Review Comment:
sg! deleted
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2112509933
##########
LogicalTypes.md:
##########
@@ -539,6 +543,19 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### INTERVAL_YEAR_MONTH
+`INTERVAL_YEAR_MONTH` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### INTERVAL_DAY_TIME
+`INTERVAL_DAY_TIME` is used to represent a day-time time interval, such as
+`5 days, 10 hours and 30 minutes`. It must annotate and 16-byte `FIXED_LEN_BYTE_ARRAY`
Review Comment:
We need a line that is like "As in ansi standard ... day_time [link or citiation) a day is always considered to be 24 hours (equivalent to X nanos)"
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2165071825
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
+ * of nanoseconds. The value can be negative to indicate a backward duration.
+ *
+ * Allowed for physical type: FIXED_LEN_BYTE_ARRAY
+ */
+struct IntervalMonthDayType {
Review Comment:
just parameterize the unit should also work. However, if the current representation is already the highest precision, it should cover the capability of representing other unit, right? i am thinking how much extra benefit it could bring by supporting different granularity.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2159983523
##########
LogicalTypes.md:
##########
@@ -539,6 +544,26 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### INTERVAL_YEAR_MONTH
+
+`INTERVAL_YEAR_MONTH` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### INTERVAL_DAY_TIME
+
+`INTERVAL_DAY_TIME` is used to represent a day-time time interval, such as
+`5 days, 10 hours and 30 minutes`. It must annotate a 16-byte `FIXED_LEN_BYTE_ARRAY`
+that stores the total number of nanoseconds representing the interval. The value is
+a signed integer and can be negative to indicate backward duration.
+The time interval is independent of any timezone.
+
+Based on the [ANSI SQL standard](https://docs.oracle.com/en/database/oracle/oracle-database/19/sqlrf/Data-Types.html#GUID-7690645A-0EE3-46CA-90DE-C96DF5A01F8F) definition of the INTERVAL data type and fields values,
+an interval of 1 day is equivalent to 24 hours, regardless of the specific number of
Review Comment:
I didn't make that suggestion, to be clear ^_^ but I do think it would help to be clear (somewhere) about whether each type is a calendar interval/relative delta or a duration/absolute time delta
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2155719049
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
+ * of nanoseconds. The value can be negative to indicate a backward duration.
+ *
+ * Allowed for physical type: FIXED_LEN_BYTE_ARRAY
Review Comment:
one issue with this is it potentially limits encodings, I wonder if we should be defining a primitive int128 physical type, or we might need to just special case some encodings.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add YEAR_MONTH_INTERVAL and DURATION types [parquet-format]
Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2237416248
##########
LogicalTypes.md:
##########
@@ -539,6 +544,30 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### YEAR_MONTH_INTERVAL
+
+`YEAR_MONTH_INTERVAL` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+
+While ANSI SQL systems typically restrict supported intervals to a range of
+±10,000 years and enforce this constraint internally, the Parquet format
+does not impose any limitations on the interval values that may be stored.
+
+#### DURATION
+
+`DURATION` is used to represent a span of time, such as `5 days`. It must
+annotate an `int64` value that stores the total number of time units for the
+duration. The value is a signed integer and can be negative to indicate backward
Review Comment:
i updated it to
```
The value is a signed integer, where a negative value indicates the
duration moves backward in time (e.g., -5 days means going backward for 5 days).
```
what do you think about the current description?
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2157795592
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
> Ok, got it. I suppose the strictly closest Arrow type, if there were one, would be 128 bit duration then (and I suppose we would want to define int128 while we're at it).
I think this is a general open question across the stack, including Parquet.
> Also, do we really need to support 10k years? Is there ever a use case that someone actually needs an interval for that much time?
SQL standards ....
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2162697458
##########
LogicalTypes.md:
##########
@@ -521,7 +521,12 @@ as shown below.
</tr>
</table>
-### INTERVAL
+### Interval types
+
+#### INTERVAL
+
+`INTERVAL` is *deprecated*. Please use `INTERVAL_YEAR_MONTH` and `INTERVAL_DAY_TIME`
Review Comment:
```
The new type is less trivially convertible from Arrow's month_day_nano, though.
```
I think the problem exists regardless whether have two types "YearMonthInterval" and "DayTimeInterval", or just have MonthDayNano type. Since the number of days in a month changes, converting from MonthDayNano to YearMonthInterval or DayTimeInterval is not possible unless we force that either the Month component is empty or the day nano components are empty.
If we want to provide full interoperability across different engines with different types, we might eventually need to support all three types.
##########
LogicalTypes.md:
##########
@@ -539,6 +544,26 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### INTERVAL_YEAR_MONTH
+
+`INTERVAL_YEAR_MONTH` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### INTERVAL_DAY_TIME
+
+`INTERVAL_DAY_TIME` is used to represent a day-time time interval, such as
+`5 days, 10 hours and 30 minutes`. It must annotate a 16-byte `FIXED_LEN_BYTE_ARRAY`
+that stores the total number of nanoseconds representing the interval. The value is
Review Comment:
The name is mainly choose based on the SQL Standard, and which is also close to the type name used by engines like spark. I think the type name doesn't have to be exactly the same as the underlying storage used. Similarly the YearMonthInterval type stores the total number of months.
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
@lidavidm Regarding to the point about "can we unify DayTimeInterval and MonthDayNano", Is the suggestion that we can just have MonthDayNano type, and DayTimeIntervalType will only not have month component?
If that is the case, it does sounds more complicated to me, because that two type is only interoperable when the month component of MonthDayNano is empty.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add YEAR_MONTH_INTERVAL and DURATION types [parquet-format]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2161108287
##########
LogicalTypes.md:
##########
@@ -539,6 +544,26 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### INTERVAL_YEAR_MONTH
+
+`INTERVAL_YEAR_MONTH` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### INTERVAL_DAY_TIME
+
+`INTERVAL_DAY_TIME` is used to represent a day-time time interval, such as
Review Comment:
Have you considered introducing a type parameter like `unit` to make it possible to use physical types like int32 or int64 for shorter storage overhead?
##########
LogicalTypes.md:
##########
@@ -539,6 +544,31 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### YEAR_MONTH_INTERVAL
+
+`YEAR_MONTH_INTERVAL` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+
+While ANSI SQL systems typically restrict supported intervals to a range of
+±10,000 years and enforce this constraint internally, the Parquet format
+does not impose any limitations on the interval values that may be stored.
+
+#### DURATION
+
+`DURATION` is used to represent a span of time, such as `5 days`. It must
+annotate an `int64` value that stores the total number of time units for the
+duration. The value is a signed integer, where a negative value indicates the
+duration moves backward in time (e.g., -5 days means going backward for 5 days).
+The duration is purely a measure of time and is independent of any time zone.
+
+The `DURATION` type takes `unit` as a parameter, and the value must be one of
+`MILLIS`, `MICROS` or `NANOS`.
Review Comment:
```suggestion
`MILLIS`, `MICROS` or `NANOS`.
```
##########
LogicalTypes.md:
##########
@@ -539,6 +544,31 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### YEAR_MONTH_INTERVAL
+
+`YEAR_MONTH_INTERVAL` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+
+While ANSI SQL systems typically restrict supported intervals to a range of
+±10,000 years and enforce this constraint internally, the Parquet format
+does not impose any limitations on the interval values that may be stored.
+
+#### DURATION
+
+`DURATION` is used to represent a span of time, such as `5 days`. It must
+annotate an `int64` value that stores the total number of time units for the
+duration. The value is a signed integer, where a negative value indicates the
+duration moves backward in time (e.g., -5 days means going backward for 5 days).
+The duration is purely a measure of time and is independent of any time zone.
+
+The `DURATION` type takes `unit` as a parameter, and the value must be one of
+`MILLIS`, `MICROS` or `NANOS`.
+
+`Duration` can be used to represent DayTime Intervals as defined by ANSI SQL. In
Review Comment:
```suggestion
`DURATION` can be used to represent day-time interval as defined by ANSI SQL. In
```
Just to be consistent with above.
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,30 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
Review Comment:
```suggestion
* The data is stored as a 4-byte signed integer which represents the number
```
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add YEAR_MONTH_INTERVAL and DURATION types [parquet-format]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2237518234
##########
LogicalTypes.md:
##########
@@ -539,6 +544,30 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### YEAR_MONTH_INTERVAL
+
+`YEAR_MONTH_INTERVAL` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+
+While ANSI SQL systems typically restrict supported intervals to a range of
+±10,000 years and enforce this constraint internally, the Parquet format
+does not impose any limitations on the interval values that may be stored.
+
+#### DURATION
+
+`DURATION` is used to represent a span of time, such as `5 days`. It must
+annotate an `int64` value that stores the total number of time units for the
+duration. The value is a signed integer and can be negative to indicate backward
Review Comment:
Sounds good to me
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2156106798
##########
LogicalTypes.md:
##########
@@ -539,6 +544,26 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### INTERVAL_YEAR_MONTH
+
+`INTERVAL_YEAR_MONTH` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### INTERVAL_DAY_TIME
+
+`INTERVAL_DAY_TIME` is used to represent a day-time time interval, such as
+`5 days, 10 hours and 30 minutes`. It must annotate a 16-byte `FIXED_LEN_BYTE_ARRAY`
+that stores the total number of nanoseconds representing the interval. The value is
+a signed integer and can be negative to indicate backward duration.
+The time interval is independent of any timezone.
+
+Based on the [ANSI SQL standard](https://docs.oracle.com/en/database/oracle/oracle-database/19/sqlrf/Data-Types.html#GUID-7690645A-0EE3-46CA-90DE-C96DF5A01F8F) definition of the INTERVAL data type and fields values,
+an interval of 1 day is equivalent to 24 hours, regardless of the specific number of
Review Comment:
@lidavidm you are correct. SQL standard calls this type interval which is where the name comes from. But for DayTime the SQL standard appears to make the distinction somewhat meaningless.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2158096228
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
> For example, looking at the [CalendarInterval](https://github.com/apache/spark/blob/master/common/unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java) spark looks like they almost match the arrow MonthDayNano (they use micros instead of nanos).
int64 for microseconds aligns with the 10K years. The arrow choice for Nano's was somewhat arbitrary IIRC to align with the finest granularity time in Arrow at the the time. I think it was likely a pragmatic choice at the time and we haven't had complaints about it to my knowledge (although I think Interval in general is fairly esoteric type I think).
> Even using an int32 of months for the YearToMonth interval doesn't match the SQL standard exactly, but spark and many other columnar compute engines use int32 instead of int64.
Can you elaborate on this, which part says it is an int64. an int32 easily holds +/- 10K years when expressed in months?
> I just think that there's more benefit to prioritizing format compatibility and reducing/avoiding conversions then to necessarily match the SQL standard. That's my personal opinion at least. Thoughts?
So I think there are potentially 2 problems to address in terms of compatibility:
1. On some level, both Date Time and Year-month intervals are real world types, and engines will want to write them (e.g. Spark added them in 4.0.0, and I would need to do some JIRA digging but I thought the intent was to replace CalendarInterval). We need some way to make an unambiguous recovery of the types from parquet. I think this is solvable with appropriate metadata, regardless of encoding, so not really a blocker.
2. Being able to store the full range of the SQL standard. Pragmatically, I think the ranges provided by Arrow's MonthDayNanos type meet most practical purposes, but I can also see the flip-side of users getting frustrated when they can't store something that the SQL engine supports in memory but can't be persisted.
A strawman proposal, if we really wanted to keep three distinct types we could encode them all in MonthDayNanos (to shred or not becomes another question with some design issues as well, I think it likely overlaps with DecFloat). Then the metadata at the parquet level could look like:
```
enum IntervalTypeRepresentation {
YearMonth - Only the Month field is ever set
DayTime - The full range of nanoseconds is used if possible. If the range exceeds the representable int64 nanos range, then
day field + nanos are used. In the latter case Nanosecond's are always sub-day and day is always interpreted as 86,400,000,000 nanoseconds.
Calendar - All fields are indepedent (e.g. match Arrow semantics exactly)
}
```
Translating to arrow could go through any of the defined interval types (subject to truncation).
Problems with this approach off the top of my head:
1. It feels inelegant/complicated (in theory DayTime intervals have a sort order and the sorting becomes non-trivial)
2. will likely slightly higher computation overhead for deltas exceeding nano second range
3. I'm still not really sure it gets us fully to Arrow compatibility because of the fallback nature for Day Field for Nanos.
@RussellSpitzer @gh-yzou thoughts here?
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2155715741
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
signed little
```suggestion
* The data is stored as a 16-byte signed little endian value, which represents the number
```
? Also, consider providing some hex strings with corresponding nano-second count to avoid ambiguity.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2156148392
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
> Nanosecond timestamps are 8 byte though right? So it's not just a trivial addition (if we're using 16 bytes here)
Yes you are correct, but for most cases I imagine the 8 most significant bytes will be sign extension which is likely slightly cheaper to check and mask out for computations.
> MonthDayNano has a 32-bit months field and 64-bit nanos field which together I would think gives you full precision for 10,000 years? (As the spec says there is no constraint on how many seconds are in the nanos field.)
Taken in aggregate then give you 10,000 years. I was referring to only the nanosecond portion (Arrow specifically calls out that each field is independent). Days was specifically in Arrow where meant to be a "calendar" type which might not be 24 hours (which is what postgres does) so the 10K year coverage using Arrow definitions would be slightly different semantics, then this representation.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2138486192
##########
LogicalTypes.md:
##########
@@ -521,7 +521,11 @@ as shown below.
</tr>
</table>
-### INTERVAL
+### INTERVAL types
Review Comment:
updated
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2158027570
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
> (Though ideally, I would still prefer to align with the Arrow types if possible 🙂)
I haven't given any thought on how to make all the requirements (SQL standards + arrow types) work together, if you have suggestions I'm sure they would be welcome.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2161113757
##########
LogicalTypes.md:
##########
@@ -521,7 +521,12 @@ as shown below.
</tr>
</table>
-### INTERVAL
+### Interval types
+
+#### INTERVAL
+
+`INTERVAL` is *deprecated*. Please use `INTERVAL_YEAR_MONTH` and `INTERVAL_DAY_TIME`
Review Comment:
Ok, I read your comment there and I think you're right. The new type is less trivially convertible from Arrow's month_day_nano, though.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add YEAR_MONTH_INTERVAL and DURATION types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2231673045
##########
LogicalTypes.md:
##########
@@ -539,6 +544,27 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### YEAR_MONTH_INTERVAL
+
+`YEAR_MONTH_INTERVAL` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### DURATION
+
+`DURATION` is used to represent a span of time, such as `5 days`. It must
+annotate an `int64` value that stores the total number of time units for the
+duration. The value is a signed integer and can be negative to indicate backward duration.
+The duration is purely a measure of time and is independent of any time zone.
+
+The `DURATION` type takes `unit` as a parameter, and the value must be one of
+`MILLIS`, `MICROS` or `NANOS`.
+
+Duration of 1 day is defined exactly of 24 hours, regardless of the specific number of
Review Comment:
I'm not sure this makes sense without mentioning, that Durations support the ANSI SQL definition of DAY_TIME. we might want to rephrase a little bit.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add YEAR_MONTH_INTERVAL and DURATION types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2231680324
##########
LogicalTypes.md:
##########
@@ -539,6 +544,27 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### YEAR_MONTH_INTERVAL
+
+`YEAR_MONTH_INTERVAL` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
Review Comment:
I think we can delete this line?
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add YEAR_MONTH_INTERVAL and DURATION types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2231682666
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,30 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct YearMonthIntervalType {
+}
+
+/**
+ * Day-Time Interval logical type annotation
Review Comment:
```suggestion
* Duration logical type annotation
```
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2113015000
##########
LogicalTypes.md:
##########
@@ -539,6 +543,19 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### INTERVAL_YEAR_MONTH
+`INTERVAL_YEAR_MONTH` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### INTERVAL_DAY_TIME
Review Comment:
```suggestion
#### INTERVAL_DAY_TIME
```
##########
LogicalTypes.md:
##########
@@ -521,7 +521,11 @@ as shown below.
</tr>
</table>
-### INTERVAL
+### INTERVAL types
+
+#### INTERVAL
+`INTERVAL` is *deprecated*. Please use `INTERVAL_YEAR_MONTH` and `INTERVAL_DAY_TIME`
+as a more precise representation per [ANSI SQL Standard](https://www.ibm.com/docs/en/informix-servers/14.10.0?topic=types-ansi-sql-standards-datetime-interval-values).
Review Comment:
Is there a more official link rather than an IBM documentation?
##########
LogicalTypes.md:
##########
@@ -521,7 +521,11 @@ as shown below.
</tr>
</table>
-### INTERVAL
+### INTERVAL types
Review Comment:
```suggestion
### Interval types
```
##########
src/main/thrift/parquet.thrift:
##########
@@ -494,6 +517,10 @@ union LogicalType {
16: VariantType VARIANT // no compatible ConvertedType
17: GeometryType GEOMETRY // no compatible ConvertedType
18: GeographyType GEOGRAPHY // no compatible ConvertedType
+
+ // INTERVAL types
Review Comment:
There is a section about `ColumnOrder` where we need to add a comment for the new interval types.
##########
LogicalTypes.md:
##########
@@ -521,7 +521,11 @@ as shown below.
</tr>
</table>
-### INTERVAL
+### INTERVAL types
+
+#### INTERVAL
Review Comment:
```suggestion
#### INTERVAL
```
##########
LogicalTypes.md:
##########
@@ -539,6 +543,19 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### INTERVAL_YEAR_MONTH
Review Comment:
```suggestion
#### INTERVAL_YEAR_MONTH
```
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2159819199
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
Review Comment:
That is a good question, i believe spark and trino today doesn't really place any strict check on the range, it is mainly limited by the storage representation. If we want to standardize across SQL engines, it might be more beneficial to stay with the SQL standard limit to avoid potential problem during sharing, such as overflow etc.
However, I also want to hear what other people think here cc @RussellSpitzer
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2160255294
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
> Short answer is MonthDayNanos more closely mimics Postgres's notion of interval, where each field is separate (so a day is not guaranteed to be 24 hours).
MonthDayNanos is a data type, it does not mandate particular semantics. You can define interval addition however you want on top of it.
(but, of course, making this new type a 16-byte nanosecond field encourages different semantics...)
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2156133500
##########
LogicalTypes.md:
##########
@@ -521,7 +521,11 @@ as shown below.
</tr>
</table>
-### INTERVAL
+### INTERVAL types
+
+#### INTERVAL
+`INTERVAL` is *deprecated*. Please use `INTERVAL_YEAR_MONTH` and `INTERVAL_DAY_TIME`
+as a more precise representation per [ANSI SQL Standard](https://www.ibm.com/docs/en/informix-servers/14.10.0?topic=types-ansi-sql-standards-datetime-interval-values).
Review Comment:
I wouldn't say either IBM or Oracle are definitive on the ANSI SQL standard? (The problem being that the spec is not freely available to any of us, so this becomes a "trust what I'm saying" situation w.r.t the spec's wording...)
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2158051999
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
> SQL standards ....
There's plenty in Parquet that isn't identical to SQL standards. My point is basically, do we really need to *match* SQL standards in Parquet?
For example, looking at the [CalendarInterval](https://github.com/apache/spark/blob/master/common/unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java) spark looks like they *almost* match the arrow MonthDayNano (they use micros instead of nanos).
Even using an int32 of months for the YearToMonth interval doesn't match the SQL standard exactly, but spark and many other columnar compute engines use int32 instead of int64.
I just think that there's more benefit to prioritizing format compatibility and reducing/avoiding conversions then to necessarily match the SQL standard. That's my personal opinion at least. Thoughts?
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add YEAR_MONTH_INTERVAL and DURATION types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2231678459
##########
LogicalTypes.md:
##########
@@ -539,6 +544,27 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### YEAR_MONTH_INTERVAL
+
+`YEAR_MONTH_INTERVAL` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### DURATION
+
+`DURATION` is used to represent a span of time, such as `5 days`. It must
+annotate an `int64` value that stores the total number of time units for the
+duration. The value is a signed integer and can be negative to indicate backward duration.
+The duration is purely a measure of time and is independent of any time zone.
+
+The `DURATION` type takes `unit` as a parameter, and the value must be one of
+`MILLIS`, `MICROS` or `NANOS`.
+
+Duration of 1 day is defined exactly of 24 hours, regardless of the specific number of
Review Comment:
```suggestion
Durations can be used to represent ANSI SQL's definition of DayTime Intervals. In this case, a duration of 1 day is defined exactly of 24 hours, regardless of the specific number of
```
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add YEAR_MONTH_INTERVAL and DURATION types [parquet-format]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2161108287
##########
LogicalTypes.md:
##########
@@ -539,6 +544,26 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### INTERVAL_YEAR_MONTH
+
+`INTERVAL_YEAR_MONTH` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### INTERVAL_DAY_TIME
+
+`INTERVAL_DAY_TIME` is used to represent a day-time time interval, such as
Review Comment:
Have you considered introducing a type parameter like `unit` to make it possible to use physical types like int32 or int64 for shorter storage overhead?
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add YEAR_MONTH_INTERVAL and DURATION types [parquet-format]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #496:
URL: https://github.com/apache/parquet-format/pull/496#issuecomment-3136504551
Any plan to add PoC implementation to arrow-cpp or arrow-rs to meet the 2 impls requirement? @sfc-gh-yzou
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add YEAR_MONTH_INTERVAL and DURATION types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #496:
URL: https://github.com/apache/parquet-format/pull/496#issuecomment-3130783021
@gh-yzou We typically wait for reference implementations before merging the format change (i.e. at this point we are at step 2 https://github.com/apache/parquet-format/blob/master/CONTRIBUTING.md).
As an example this is how geography was done (https://lists.apache.org/thread/s6s714c98cn9gg22mnk5nsn7xymym8xo). Variant has been handled a little bit differently because it originated outside of the project.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "sfc-gh-yzou (via GitHub)" <gi...@apache.org>.
sfc-gh-yzou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2113032317
##########
LogicalTypes.md:
##########
@@ -521,7 +521,11 @@ as shown below.
</tr>
</table>
-### INTERVAL
+### INTERVAL types
+
+#### INTERVAL
+`INTERVAL` is *deprecated*. Please use `INTERVAL_YEAR_MONTH` and `INTERVAL_DAY_TIME`
+as a more precise representation per [ANSI SQL Standard](https://www.ibm.com/docs/en/informix-servers/14.10.0?topic=types-ansi-sql-standards-datetime-interval-values).
Review Comment:
I find an oracle definition, which might be better than the IBM one. updated
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2112500887
##########
LogicalTypes.md:
##########
@@ -539,6 +543,19 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### INTERVAL_YEAR_MONTH
+`INTERVAL_YEAR_MONTH` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### INTERVAL_DAY_TIME
+`INTERVAL_DAY_TIME` is used to represent a day-time time interval, such as
+`5 days, 10 hours and 30 minutes`. It must annotate and 16-byte `FIXED_LEN_BYTE_ARRAY`
Review Comment:
```suggestion
`5 days, 10 hours and 30 minutes`. It must annotate a 16-byte `FIXED_LEN_BYTE_ARRAY`
```
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2156106798
##########
LogicalTypes.md:
##########
@@ -539,6 +544,26 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### INTERVAL_YEAR_MONTH
+
+`INTERVAL_YEAR_MONTH` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### INTERVAL_DAY_TIME
+
+`INTERVAL_DAY_TIME` is used to represent a day-time time interval, such as
+`5 days, 10 hours and 30 minutes`. It must annotate a 16-byte `FIXED_LEN_BYTE_ARRAY`
+that stores the total number of nanoseconds representing the interval. The value is
+a signed integer and can be negative to indicate backward duration.
+The time interval is independent of any timezone.
+
+Based on the [ANSI SQL standard](https://docs.oracle.com/en/database/oracle/oracle-database/19/sqlrf/Data-Types.html#GUID-7690645A-0EE3-46CA-90DE-C96DF5A01F8F) definition of the INTERVAL data type and fields values,
+an interval of 1 day is equivalent to 24 hours, regardless of the specific number of
Review Comment:
@lidavidm you are correct. SQL standard calls this type interval which is where the name comes from. But for DayTime the SQL standard appears to make the distinction somewhat meaningless. For year-month there is a clear semantic difference.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2157692520
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
Also, do we really need to support 10k years? Is there ever a use case that someone actually needs an interval for that much time?
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2160030278
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
IMO, it's less "We need a month-day-nano type" and more "can we unify DayTimeInterval and MonthDayNano"? It sounds like ~maybe, if we're willing to accept a somewhat more complicated representation. Or maybe the complexity should be pushed to the Arrow side (with the encoding Micah suggests). But that's why I suggested making sure we open a dialogue with the Arrow community too (since the encoding is somewhat fraught: again, there's a mismatch between ANSI SQL having what is really a duration or absolute time difference, and MonthDayNano being an "actual" interval, so I'm not quite convinced we want to translate in this way).
Given (for instance) PyIceberg, iceberg-rust, and iceberg-cpp use Arrow (to different extents), I think having a mapping with clearly defined semantics isn't an abstract concern for Iceberg implementations and the question shouldn't just be dropped.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2162769933
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
+ * of nanoseconds. The value can be negative to indicate a backward duration.
+ *
+ * Allowed for physical type: FIXED_LEN_BYTE_ARRAY
+ */
+struct IntervalMonthDayType {
Review Comment:
Actually, I made a mistake before, this the type is not really MonthDayType, it is actually DayTimeIntervalType, the types doesn't have a Month component. Is the suggestion to have name like "IntervalDayNanoType" ? I think the problem is if we call it DayNano is that in the future we might need to introduce type like DayMills, DayMirco for different precision usage, which could be quite tedious. I think it might be cleaner to have DayTime and then introduce scale and precision parameter for different granularity in the future.
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
updated, also added and example
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2156159721
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
Ok, got it. I suppose the strictly closest Arrow type, if there were one, would be 128 bit duration then (and I suppose we would want to define int128 while we're at it).
I think if we're going to do this, we should kick off a parallel discussion in Arrow (and maybe RAPIDS/cuDF) for a potential new (extension) type since otherwise we're won't be able to actually represent the new type in (for instance) all our client libraries.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2156160160
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
(Though ideally, I would still prefer to align with the Arrow types if possible :slightly_smiling_face:)
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add YEAR_MONTH_INTERVAL and DURATION types [parquet-format]
Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on PR #496:
URL: https://github.com/apache/parquet-format/pull/496#issuecomment-3229819142
Hi @wgtmac sorry for the late reply! Got distracted by some other projects, the current plan is to add java and cpp implementation for the interval type, no specific plan for arrow yet. Perhaps, i can swap the cpp support to arrow-cpp if that is more useful?
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2159820680
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
+ * of nanoseconds. The value can be negative to indicate a backward duration.
+ *
+ * Allowed for physical type: FIXED_LEN_BYTE_ARRAY
+ */
+struct IntervalMonthDayType {
Review Comment:
I think it would be more beneficial to keep IntervalDayTimeType to be more consistent with the SQL standard. For the granularity, i think we can potentially introduce scale and precision for engine to interpret, what do you think?
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2164307577
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
+ * of nanoseconds. The value can be negative to indicate a backward duration.
+ *
+ * Allowed for physical type: FIXED_LEN_BYTE_ARRAY
+ */
+struct IntervalMonthDayType {
Review Comment:
Instead of using "scale" and "precision", can we instead just parameterize the unit (Milli, Micro, Nano, etc.)?
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2160255702
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
+ * of nanoseconds. The value can be negative to indicate a backward duration.
+ *
+ * Allowed for physical type: FIXED_LEN_BYTE_ARRAY
Review Comment:
AFAIU, only DELTA_BINARY_PACKED would come on top of a hypothetical INT128 type... assuming it's ever implemented.
OTOH, FLBA gives you several encodings for free that are *already* implemented. In particular, BYTE_STREAM_SPLIT should be quite efficient at making this new data type more compressible.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2160254238
##########
LogicalTypes.md:
##########
@@ -521,7 +521,12 @@ as shown below.
</tr>
</table>
-### INTERVAL
+### Interval types
+
+#### INTERVAL
+
+`INTERVAL` is *deprecated*. Please use `INTERVAL_YEAR_MONTH` and `INTERVAL_DAY_TIME`
Review Comment:
Why deprecated? Parquet is not limited to SQL workloads.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2161081487
##########
LogicalTypes.md:
##########
@@ -521,7 +521,12 @@ as shown below.
</tr>
</table>
-### INTERVAL
+### Interval types
+
+#### INTERVAL
+
+`INTERVAL` is *deprecated*. Please use `INTERVAL_YEAR_MONTH` and `INTERVAL_DAY_TIME`
Review Comment:
From the discussion at https://docs.google.com/document/d/12ghQxWxyAhSQeZyy0IWiwJ02gTqFOgfYm8x851HZFLk, I think the existing Parquet interval types are not well-thought and not adopted widely (may be biased).
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2156106798
##########
LogicalTypes.md:
##########
@@ -539,6 +544,26 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### INTERVAL_YEAR_MONTH
+
+`INTERVAL_YEAR_MONTH` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### INTERVAL_DAY_TIME
+
+`INTERVAL_DAY_TIME` is used to represent a day-time time interval, such as
+`5 days, 10 hours and 30 minutes`. It must annotate a 16-byte `FIXED_LEN_BYTE_ARRAY`
+that stores the total number of nanoseconds representing the interval. The value is
+a signed integer and can be negative to indicate backward duration.
+The time interval is independent of any timezone.
+
+Based on the [ANSI SQL standard](https://docs.oracle.com/en/database/oracle/oracle-database/19/sqlrf/Data-Types.html#GUID-7690645A-0EE3-46CA-90DE-C96DF5A01F8F) definition of the INTERVAL data type and fields values,
+an interval of 1 day is equivalent to 24 hours, regardless of the specific number of
Review Comment:
@lidavidm you are correct. SQL standard calls this type interval which is where the name comes from.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add YEAR_MONTH_INTERVAL and DURATION types [parquet-format]
Posted by "sfc-gh-yzou (via GitHub)" <gi...@apache.org>.
sfc-gh-yzou commented on PR #496:
URL: https://github.com/apache/parquet-format/pull/496#issuecomment-3130834671
@emkornfield Thanks a lot for the pointer! I will start the reference implementation for java.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2167123991
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
One thing i'm a little worried about with the Arrow definition for MonthDayNano is the handling of bounds. According to the arrow spec the Nano and Day portions are allowed to vary in signs. Wouldn't this make calculations of min-max values a bit more confusing? I think if we did go that route, and i'm not sure we should, but I think we would have to add a few other limitations which would diverge a bit 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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2112516086
##########
LogicalTypes.md:
##########
@@ -539,6 +543,19 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### INTERVAL_YEAR_MONTH
+`INTERVAL_YEAR_MONTH` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### INTERVAL_DAY_TIME
+`INTERVAL_DAY_TIME` is used to represent a day-time time interval, such as
+`5 days, 10 hours and 30 minutes`. It must annotate and 16-byte `FIXED_LEN_BYTE_ARRAY`
Review Comment:
I think we still have to decide whether we also just want to introduce at 128bit int primitive type, but this is ok for now and we can even shape it like a 128bit int primitive ...
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2113018023
##########
LogicalTypes.md:
##########
@@ -539,6 +543,19 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### INTERVAL_YEAR_MONTH
+`INTERVAL_YEAR_MONTH` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### INTERVAL_DAY_TIME
+`INTERVAL_DAY_TIME` is used to represent a day-time time interval, such as
+`5 days, 10 hours and 30 minutes`. It must annotate and 16-byte `FIXED_LEN_BYTE_ARRAY`
Review Comment:
updated
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "sfc-gh-yzou (via GitHub)" <gi...@apache.org>.
sfc-gh-yzou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2113030890
##########
LogicalTypes.md:
##########
@@ -521,7 +521,11 @@ as shown below.
</tr>
</table>
-### INTERVAL
+### INTERVAL types
+
+#### INTERVAL
Review Comment:
added new line
##########
LogicalTypes.md:
##########
@@ -539,6 +543,19 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### INTERVAL_YEAR_MONTH
Review Comment:
added new line
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add YEAR_MONTH_INTERVAL and DURATION types [parquet-format]
Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on PR #496:
URL: https://github.com/apache/parquet-format/pull/496#issuecomment-3119523306
@RussellSpitzer @lidavidm @pitrou @wgtmac @zeroshade I updated the PR based on the discussion result on this thread https://lists.apache.org/thread/n8jdft4mltdcf91v7t8qf1hz5cg8nbnz, please help take one more look. Thanks a lot for helping on the review!
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add YEAR_MONTH_INTERVAL and DURATION types [parquet-format]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2237003206
##########
LogicalTypes.md:
##########
@@ -539,6 +544,30 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### YEAR_MONTH_INTERVAL
+
+`YEAR_MONTH_INTERVAL` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+
+While ANSI SQL systems typically restrict supported intervals to a range of
+±10,000 years and enforce this constraint internally, the Parquet format
+does not impose any limitations on the interval values that may be stored.
+
+#### DURATION
+
+`DURATION` is used to represent a span of time, such as `5 days`. It must
+annotate an `int64` value that stores the total number of time units for the
+duration. The value is a signed integer and can be negative to indicate backward
Review Comment:
```suggestion
duration. The value is a signed integer and can be negative to indicate a backwards
```
Minor grammar fix^
But I am wondering if we have a better way to describe what a negative duration represents.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2155724835
##########
LogicalTypes.md:
##########
@@ -539,6 +544,26 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### INTERVAL_YEAR_MONTH
+
+`INTERVAL_YEAR_MONTH` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### INTERVAL_DAY_TIME
+
+`INTERVAL_DAY_TIME` is used to represent a day-time time interval, such as
+`5 days, 10 hours and 30 minutes`. It must annotate a 16-byte `FIXED_LEN_BYTE_ARRAY`
+that stores the total number of nanoseconds representing the interval. The value is
+a signed integer and can be negative to indicate backward duration.
+The time interval is independent of any timezone.
+
+Based on the [ANSI SQL standard](https://docs.oracle.com/en/database/oracle/oracle-database/19/sqlrf/Data-Types.html#GUID-7690645A-0EE3-46CA-90DE-C96DF5A01F8F) definition of the INTERVAL data type and fields values,
+an interval of 1 day is equivalent to 24 hours, regardless of the specific number of
Review Comment:
This definition also turns this into what other systems often call a "duration" not an "interval" right? For instance this would map to pandas/datetime Timedelta and not pandas DateOffset.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2155715169
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
+ * of nanoseconds. The value can be negative to indicate a backward duration.
+ *
+ * Allowed for physical type: FIXED_LEN_BYTE_ARRAY
+ */
+struct IntervalMonthDayType {
Review Comment:
I'd suggest either naming this with nanoseconds or parameterizing it in case we want a different granularity in the future.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2156105806
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
Short answer is MonthDayNanos more closely mimics Postgres's notion of interval, where each field is separate (so a day is not guaranteed to be 24 hours). Apparently, according the SQL spec (which is not publicly available, and I don't currently have access to and at least some [other DB providers](https://stackoverflow.com/questions/61505068/is-interval-1-day-always-equal-to-interval-24-hours) also follow the convention) is that a day is always 24 hours/86400 seconds as documented.
The main difference that arise from the representations are:
1. MonthDayNanos in arrow allows for days that are not 24 hours, and you can have more the 86400 seconds in the nanos field. One short-coming of the arrow representation is it can't represent +/- 10000 years at nanosecond precision which is typical in SQL.
2. This representations makes it so everything can be normalized. But I think a fair question is why not encode it as days + nanoseconds and do the multiplication instead of division to get backs (in this case I think it just means more math then applying nanosecond addition directly to nanosecond timestamps).
That being said, I think there might be a place for all 3 of these representations just like we have all three in arrow, if someone wants to add the third.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2156148392
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
> Nanosecond timestamps are 8 byte though right? So it's not just a trivial addition (if we're using 16 bytes here)
Yes you are correct, but for most cases I imagine the 8 most significant bytes will be sign extension which is likely slightly cheaper to check and mask out then force multiplications.
> MonthDayNano has a 32-bit months field and 64-bit nanos field which together I would think gives you full precision for 10,000 years? (As the spec says there is no constraint on how many seconds are in the nanos field.)
Taken in aggregate then give you 10,000 years. I was referring to only the nanosecond portion (Arrow specifically calls out that each field is independent). Days was specifically in Arrow where meant to be a "calendar" type which might not be 24 hours (which is what postgres does) so the 10K year coverage using Arrow definitions would be slightly different semantics, then this representation.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2160255702
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
+ * of nanoseconds. The value can be negative to indicate a backward duration.
+ *
+ * Allowed for physical type: FIXED_LEN_BYTE_ARRAY
Review Comment:
AFAIU, only DELTA_BINARY_PACKED would come on top of a hypothetical INT128 type... assuming it's ever implemented for 128-bit ints (and, if possible, implemented efficiently!).
OTOH, FLBA gives you several encodings for free that are *already* implemented. In particular, BYTE_STREAM_SPLIT should be quite efficient at making this new data type more compressible.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add YEAR_MONTH_INTERVAL and DURATION types [parquet-format]
Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on PR #496:
URL: https://github.com/apache/parquet-format/pull/496#issuecomment-3130310997
@emkornfield Thanks a lot for helping on the discussion and review! Do you think we can merge this PR? or shall we wait for a bit longer?
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add YEAR_MONTH_INTERVAL and DURATION types [parquet-format]
Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on PR #496:
URL: https://github.com/apache/parquet-format/pull/496#issuecomment-3237637916
@wgtmac yes, that is the one i refer to. Sorry, i am new to the community, I guess, we are talking about the same thing about arrow-cpp and parquet-cpp then :).
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2167201646
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
Limitations that are still valid for the Arrow MonthDayNano would still be very useful as it would still provide ~zero-copy reads in that we wouldn't need to modify/transform the values during read. Writing would just need to update the value to meet the limitations before we write.
> According to the arrow spec the Nano and Day portions are allowed to vary in signs. Wouldn't this make calculations of min-max values a bit more confusing?
It would make it a bit more complex, sure, but I don't think they would be confusing. Every value would still refer to a specific interval of time that can be compared.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2165124217
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
Basically my main goal is to have the type map cleanly onto an Arrow type. Currently this being 16 bytes makes that very hard.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2165957820
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
Review Comment:
Why would we care about alignment in a Parquet file? Most likely this will require conversion to another in-memory format 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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2167115871
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
+ * of nanoseconds. The value can be negative to indicate a backward duration.
+ *
+ * Allowed for physical type: FIXED_LEN_BYTE_ARRAY
+ */
+struct IntervalMonthDayType {
Review Comment:
My thoughts here were similar, generally we want to add different precisions is because we want to cover different ranges of possible values, but if our underlying type can support the full range we expect of other precisions it probably isn't important.
I think this is basically the rational behind some of the other interval types like in MonthDayNanos (assuming you treat days not as calendar days - not in spec but theoretically you could have done that)
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2156132587
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
Nanosecond timestamps are 8 byte though right? So it's not just a trivial addition (if we're using 16 bytes here)
> One short-coming of the arrow representation is it can't represent +/- 10000 years at nanosecond precision which is typical in SQL.
MonthDayNano has a 32-bit months field and 64-bit nanos field which together I would think gives you full precision for 10,000 years? (As the spec says there is no constraint on how many seconds are in the nanos field.)
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2165227668
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
Review Comment:
The 16 is chosen because we want to cover representation capabilities across various engines, where Oracle support a max range of +/- 999,999,999 days, which requires at lease 10 bytes. we choose 16 bytes are mainly for alignment and compatibility
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2167216062
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
I was thinking selfishly about how I could do a straight binary comparison if we don't mix signs, but if we do mix signs I have to deserialize first 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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2167220771
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
Zero-copy reads don't really exist in Parquet, except if you use no nulls, no compression, no encryption, PLAIN encoding, and you're willing to carry data in page-sized chunks.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2155718305
##########
LogicalTypes.md:
##########
@@ -539,6 +544,26 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### INTERVAL_YEAR_MONTH
+
+`INTERVAL_YEAR_MONTH` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### INTERVAL_DAY_TIME
+
+`INTERVAL_DAY_TIME` is used to represent a day-time time interval, such as
+`5 days, 10 hours and 30 minutes`. It must annotate a 16-byte `FIXED_LEN_BYTE_ARRAY`
+that stores the total number of nanoseconds representing the interval. The value is
+a signed integer and can be negative to indicate backward duration.
+The time interval is independent of any timezone.
+
+Based on the [ANSI SQL standard](https://docs.oracle.com/en/database/oracle/oracle-database/19/sqlrf/Data-Types.html#GUID-7690645A-0EE3-46CA-90DE-C96DF5A01F8F) definition of the INTERVAL data type and fields values,
+an interval of 1 day is equivalent to 24 hours, regardless of the specific number of
Review Comment:
this detail should also probably be in parquet.thrift (reference to SQL standard probably isn't strictly necessary there, just how 1 day is defined).
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2159814278
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
Hi @zeroshade and @emkornfield Thanks a lot for the feedbacks! Sorry, it took me sometime to catch up here. Thanks a lot to bring up the point about MonthDayNano.
I felt the communities has been put on a lot of effort to push standard across SQL engines (I might be wrong). From that point of view, i do feel is is important to come with a standard can ease the sharing cross different SQL engines and stay close with the SQL standard.
As for MonthDayNano, although Spark today does have a CalendarInterval type, it seems that the recommended user facing type is still YearMonthInterval and MonthDayInterval. Furthermore, i think having YearMonthInterval and DayTimeInterval doesn't really block us to also support the MonthDayNano in the near future if it is necessary.
So I think it might make more sense to support YearMonthInterval and DayTimeInterval, which probably satisfies most of the cross engine sharing use cases, and then if there is high demand for the other type, we can also considering adding it to make the type system richer. What do you think?
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2160256027
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
+ * of nanoseconds. The value can be negative to indicate a backward duration.
+ *
+ * Allowed for physical type: FIXED_LEN_BYTE_ARRAY
+ */
+struct IntervalMonthDayType {
Review Comment:
@gh-yzou Parquet is not tied to SQL, so we shouldn't feel constrained by SQL's choice of vocabulary, especially when it could be misleading or confusing.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2160030278
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
IMO, it's less "We need a month-day-nano type" and more "can we unify DayTimeInterval and MonthDayNano"? It sounds like ~maybe, if we're willing to accept a somewhat more complicated representation. Or maybe the complexity should be pushed to the Arrow side (with the encoding Micah suggests). But that's why I suggested making sure we open a dialogue with the Arrow community too (since the encoding is somewhat fraught: again, there's a mismatch between ANSI SQL having what is really a duration or absolute time difference, and MonthDayNano being an "actual" interval, so I'm not quite convinced we want to translate in this way).
Given (for instance) PyIceberg, iceberg-rust, and iceberg-cpp use Arrow (to different extents), I think having a mapping with clearly defined semantics isn't an abstract concern for Iceberg implementations and the question shouldn't just be dropped because of existing behavior of some engines.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2159820680
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
+ * of nanoseconds. The value can be negative to indicate a backward duration.
+ *
+ * Allowed for physical type: FIXED_LEN_BYTE_ARRAY
+ */
+struct IntervalMonthDayType {
Review Comment:
I think it would be more beneficial to keep IntervalMonthDayType to be more consistent with the SQL standard. For the granularity, i think we can potentially introduce scale and precision for engine to interpret, what do you think?
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2159820426
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
+ * of nanoseconds. The value can be negative to indicate a backward duration.
+ *
+ * Allowed for physical type: FIXED_LEN_BYTE_ARRAY
Review Comment:
i think my concern with int128 is that would that be just used by IntervalMonthDayType, and we might need to also introduce uint128 to be more complete. For now, I think just special case some encodings might be good enough.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2155796841
##########
src/main/thrift/parquet.thrift:
##########
@@ -461,6 +461,29 @@ struct GeographyType {
2: optional EdgeInterpolationAlgorithm algorithm;
}
+/**
+ * Year-Month Interval logical type annotation
+ *
+ * The data is stored as an 4 byte signed integer which represents the number
+ * of months associated with the time interval. The value can be negative to
+ * indicate a backward duration.
+ *
+ * Allowed for physical type: INT32
+ */
+struct IntervalYearMonthType {
+}
+
+/**
+ * Month-Day Interval logical type annotation
+ *
+ * The data is stored as a 16-byte signed value, which represents the number
Review Comment:
Is there a good reason to go with a 16-byte signed integer as opposed to using Arrow's `MonthDayNano` type? That way Parquet maintains a simple and efficient interoperability with Arrow instead of forcing a new Arrow type to be compatible
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2113019390
##########
LogicalTypes.md:
##########
@@ -539,6 +543,19 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### INTERVAL_YEAR_MONTH
+`INTERVAL_YEAR_MONTH` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### INTERVAL_DAY_TIME
+`INTERVAL_DAY_TIME` is used to represent a day-time time interval, such as
+`5 days, 10 hours and 30 minutes`. It must annotate and 16-byte `FIXED_LEN_BYTE_ARRAY`
Review Comment:
i added one more paragraph at the end of this section. There is actually no documentation explicitly stated that 1 day is equal to 24 hours, those seems indicated from the value range definition of the SQL standard, so I linked the whole SQL standard doc here.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add INTERVAL_YEAR_MONTH and INTERVAL_DAY_TIME types [parquet-format]
Posted by "sfc-gh-yzou (via GitHub)" <gi...@apache.org>.
sfc-gh-yzou commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2113031147
##########
LogicalTypes.md:
##########
@@ -539,6 +543,19 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### INTERVAL_YEAR_MONTH
+`INTERVAL_YEAR_MONTH` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
+The time duration is independent of any timezone.
+
+#### INTERVAL_DAY_TIME
Review Comment:
added new line
##########
src/main/thrift/parquet.thrift:
##########
@@ -494,6 +517,10 @@ union LogicalType {
16: VariantType VARIANT // no compatible ConvertedType
17: GeometryType GEOMETRY // no compatible ConvertedType
18: GeographyType GEOGRAPHY // no compatible ConvertedType
+
+ // INTERVAL types
Review Comment:
added
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add YEAR_MONTH_INTERVAL and DURATION types [parquet-format]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #496:
URL: https://github.com/apache/parquet-format/pull/496#discussion_r2231681749
##########
LogicalTypes.md:
##########
@@ -539,6 +544,27 @@ The sort order used for `INTERVAL` is undefined. When writing data, no min/max
statistics should be saved for this type and if such non-compliant statistics
are found during reading, they must be ignored.
+#### YEAR_MONTH_INTERVAL
+
+`YEAR_MONTH_INTERVAL` is used to represent a year-month time interval, such as
+`4 years and 6 months`. It must annotate an `int32` that stores the total number
+of months as a signed integer, which represents the interval and can be negative.
Review Comment:
```suggestion
of months as a signed integer, which represents the interval and can be negative. Some ANSI SQL systems only support intervals in the range of +/- 10,000 years, no constraint is placed on the exact value in Parquet.
```
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] Add YEAR_MONTH_INTERVAL and DURATION types [parquet-format]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #496:
URL: https://github.com/apache/parquet-format/pull/496#issuecomment-3235491318
@gh-yzou I'm just wondering what cpp implementation are you talking about. Usually we refer to parquet-cpp in the arrow-cpp repo: https://github.com/apache/arrow/tree/main/cpp/src/parquet
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org