>From Wail Alkowaileet <[email protected]>:

Attention is currently required from: Ritik Raj.

Wail Alkowaileet has posted comments on this change by Ritik Raj. ( 
https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20773?usp=email )

Change subject: [ASTERIXDB-3684][STO] Support temporal types in Column
......................................................................


Patch Set 5:

(6 comments)

Commit Message:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20773/comment/7e775fca_91db5975?usp=email
 :
PS5, Line 23: DURATION and
            : INTERVAL
Can you elaborate on why is that here?


File 
asterixdb/asterix-column/src/main/java/org/apache/asterix/column/bytes/encoder/ParquetDeltaBinaryPackingValuesWriterForInteger.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20773/comment/35a13df8_56e4404f?usp=email
 :
PS5, Line 120: 32
Good catch!


File 
asterixdb/asterix-column/src/main/java/org/apache/asterix/column/values/reader/ColumnValueReaderFactory.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20773/comment/0eefb095_7cbe8ca1?usp=email
 :
PS5, Line 89: case DATE:
            :                 return new DateValueReader();
            :             case TIME:
            :                 return new TimeValueReader();
            :             case YEARMONTHDURATION:
            :                 return new YearMonthDurationValueReader();
            :             case DATETIME:
            :                 return new DateTimeValueReader();
            :             case DAYTIMEDURATION:
            :                 return new DayTimeDurationValueReader();
            :             case DURATION:
            :                 return new DurationValueReader();
            :             case INTERVAL:
            :                 return new IntervalValueReader();
Can any of these be a primaryKey? Then we need reader/writer w/ plain 
encoding/decoding.


File 
asterixdb/asterix-column/src/main/java/org/apache/asterix/column/values/reader/value/temporal/DurationValueReader.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20773/comment/922dbb4a_0b3ee71b?usp=email
 :
PS5, Line 78:         // Compare using Asterix duration ordering: months first, 
then day-time milliseconds.
            :         IValueReference other = o.getBytes();
            :
            :         byte[] a = value.getByteArray();
            :         int ao = value.getStartOffset();
            :         int monthsA = IntegerPointable.getInteger(a, ao);
            :         long millisA = LongPointable.getLong(a, ao + 
Integer.BYTES);
            :
            :         byte[] b = other.getByteArray();
            :         int bo = other.getStartOffset();
            :         int monthsB = IntegerPointable.getInteger(b, bo);
            :         long millisB = LongPointable.getLong(b, bo + 
Integer.BYTES);
            :
            :         int cmp = Integer.compare(monthsA, monthsB);
            :         return cmp != 0 ? cmp : Long.compare(millisA, millisB);
Minor: can we do the comparison of months before deserializing the values of 
the daytime?


File 
asterixdb/asterix-column/src/main/java/org/apache/asterix/column/values/reader/value/temporal/IntervalValueReader.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20773/comment/c1cd587a_242936ff?usp=email
 :
PS5, Line 75: intervalTimeType
Is this dynamic? Or determined upon creation of the reader? 

This is something I would discuss with the team before committing into this. I 
believe we can redesign/reformat those types (row or column) without worrying 
about being back compatible.

It is a bit ugly. It is two types merged into one.


File 
asterixdb/asterix-column/src/main/java/org/apache/asterix/column/values/writer/DurationColumnValuesWriter.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20773/comment/ae114ca5_4db99cfc?usp=email
 :
PS5, Line 55: * NOTE: Since the columnar page-zero filter stores only two 
8-byte longs, we cannot safely create an
            :  * order-preserving single-long min/max representation for 
DURATION. This writer therefore stores the values, but
            :  * uses a conservative filter that avoids pruning based on 
DURATION filters.
Cannot we do as we do w/ strings? filter using months only? At least that's how 
DurtationValueReader#compareTo() works



--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20773?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://asterix-gerrit.ics.uci.edu/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Change-Id: I1bdf63561cd5e7c73801380cf0a7c1b5be9c9248
Gerrit-Change-Number: 20773
Gerrit-PatchSet: 5
Gerrit-Owner: Ritik Raj <[email protected]>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Ritik Raj <[email protected]>
Gerrit-CC: Wail Alkowaileet <[email protected]>
Gerrit-Attention: Ritik Raj <[email protected]>
Gerrit-Comment-Date: Wed, 14 Jan 2026 08:02:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No

Reply via email to