trxcllnt commented on code in PR #37341:
URL: https://github.com/apache/arrow/pull/37341#discussion_r1303707443
##########
js/src/visitor/get.ts:
##########
@@ -279,6 +285,25 @@ const getIntervalYearMonth = <T extends
IntervalYearMonth>({ values }: Data<T>,
return int32s;
};
+/** @ignore */
+const getDurationSecond = <T extends DurationSecond>({ values }: Data<T>,
index: number): T['TValue'] => values[index];
+/** @ignore */
+const getDurationMillisecond = <T extends DurationMillisecond>({ values }:
Data<T>, index: number): T['TValue'] => values[index];
+/** @ignore */
+const getDurationMicrosecond = <T extends DurationMicrosecond>({ values }:
Data<T>, index: number): T['TValue'] => values[index];
+/** @ignore */
+const getDurationNanosecond = <T extends DurationNanosecond>({ values }:
Data<T>, index: number): T['TValue'] => values[index];
+/* istanbul ignore next */
+/** @ignore */
+const getDuration = <T extends Duration>(data: Data<T>, index: number):
T['TValue'] => {
+ switch (data.type.unit) {
+ case TimeUnit.SECOND: return getDurationSecond(data as
Data<DurationSecond>, index);
+ case TimeUnit.MILLISECOND: return getDurationMillisecond(data as
Data<DurationMillisecond>, index);
+ case TimeUnit.MICROSECOND: return getDurationMicrosecond(data as
Data<DurationMicrosecond>, index);
+ case TimeUnit.NANOSECOND: return getDurationNanosecond(data as
Data<DurationNanosecond>, index);
+ }
+};
+
Review Comment:
Since the implementation for these is all the same, can we collapse them
into one definition?
```suggestion
/* istanbul ignore next */
/** @ignore */
const getDuration = <T extends Duration>(data: Data<T>, index: number):
T['TValue'] => values[index];
```
##########
js/src/visitor/set.ts:
##########
@@ -364,6 +390,11 @@ SetVisitor.prototype.visitDictionary =
wrapSet(setDictionary);
SetVisitor.prototype.visitInterval = wrapSet(setIntervalValue);
SetVisitor.prototype.visitIntervalDayTime = wrapSet(setIntervalDayTime);
SetVisitor.prototype.visitIntervalYearMonth = wrapSet(setIntervalYearMonth);
+SetVisitor.prototype.visitDuration = wrapSet(setDuration);
+SetVisitor.prototype.visitDurationSecond = wrapSet(setDurationSecond);
+SetVisitor.prototype.visitDurationMillisecond =
wrapSet(setDurationMillisecond);
+SetVisitor.prototype.visitDurationMicrosecond =
wrapSet(setDurationMicrosecond);
+SetVisitor.prototype.visitDurationNanosecond = wrapSet(setDurationNanosecond);
Review Comment:
```suggestion
SetVisitor.prototype.visitDuration = wrapSet(setDuration);
SetVisitor.prototype.visitDurationSecond = wrapSet(setDuration);
SetVisitor.prototype.visitDurationMillisecond = wrapSet(setDuration);
SetVisitor.prototype.visitDurationMicrosecond = wrapSet(setDuration);
SetVisitor.prototype.visitDurationNanosecond = wrapSet(setDuration);
```
##########
js/src/visitor/get.ts:
##########
@@ -328,6 +353,11 @@ GetVisitor.prototype.visitDictionary =
wrapGet(getDictionary);
GetVisitor.prototype.visitInterval = wrapGet(getInterval);
GetVisitor.prototype.visitIntervalDayTime = wrapGet(getIntervalDayTime);
GetVisitor.prototype.visitIntervalYearMonth = wrapGet(getIntervalYearMonth);
+GetVisitor.prototype.visitDuration = wrapGet(getDuration);
+GetVisitor.prototype.visitDurationSecond = wrapGet(getDurationSecond);
+GetVisitor.prototype.visitDurationMillisecond =
wrapGet(getDurationMillisecond);
+GetVisitor.prototype.visitDurationMicrosecond =
wrapGet(getDurationMicrosecond);
+GetVisitor.prototype.visitDurationNanosecond = wrapGet(getDurationNanosecond);
Review Comment:
Same question here.
```suggestion
GetVisitor.prototype.visitDuration = wrapGet(getDuration);
GetVisitor.prototype.visitDurationSecond = wrapGet(getDuration);
GetVisitor.prototype.visitDurationMillisecond = wrapGet(getDuration);
GetVisitor.prototype.visitDurationMicrosecond = wrapGet(getDuration);
GetVisitor.prototype.visitDurationNanosecond = wrapGet(getDuration);
```
##########
js/src/visitor/set.ts:
##########
@@ -308,6 +314,26 @@ export const setIntervalDayTime = <T extends
IntervalDayTime>({ values }: Data<T
/** @ignore */
export const setIntervalYearMonth = <T extends IntervalYearMonth>({ values }:
Data<T>, index: number, value: T['TValue']): void => { values[index] =
(value[0] * 12) + (value[1] % 12); };
+/** @ignore */
+export const setDurationSecond = <T extends DurationSecond>({ values }:
Data<T>, index: number, value: T['TValue']): void => { values[index] = value; };
+/** @ignore */
+export const setDurationMillisecond = <T extends DurationMillisecond>({ values
}: Data<T>, index: number, value: T['TValue']): void => { values[index] =
value; };
+/** @ignore */
+export const setDurationMicrosecond = <T extends DurationMicrosecond>({ values
}: Data<T>, index: number, value: T['TValue']): void => { values[index] =
value; };
+/** @ignore */
+export const setDurationNanosecond = <T extends DurationNanosecond>({ values
}: Data<T>, index: number, value: T['TValue']): void => { values[index] =
value; };
+/* istanbul ignore next */
+/** @ignore */
+export const setDuration = <T extends Duration>(data: Data<T>, index: number,
value: T['TValue']): void => {
+ switch (data.type.unit) {
+ case TimeUnit.SECOND: return setDurationSecond(data as
Data<DurationSecond>, index, value as DurationSecond['TValue']);
+ case TimeUnit.MILLISECOND: return setDurationMillisecond(data as
Data<DurationMillisecond>, index, value as DurationMillisecond['TValue']);
+ case TimeUnit.MICROSECOND: return setDurationMicrosecond(data as
Data<DurationMicrosecond>, index, value as DurationMicrosecond['TValue']);
+ case TimeUnit.NANOSECOND: return setDurationNanosecond(data as
Data<DurationNanosecond>, index, value as DurationNanosecond['TValue']);
+ }
+};
+
+
Review Comment:
```suggestion
/* istanbul ignore next */
/** @ignore */
export const setDuration = <T extends Duration>(data: Data<T>, index:
number, value: T['TValue']): void => { values[index] = value; };
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
