You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "domoritz (via GitHub)" <gi...@apache.org> on 2024/03/29 15:49:28 UTC
[PR] GH-40891: [JS] Store Dates as TimestampMillisecond [arrow]
domoritz opened a new pull request, #40892:
URL: https://github.com/apache/arrow/pull/40892
Tested with
```ts
const date = new Date("2023-03-29T12:34:56Z");
console.log("original", date)
console.log("=> vec")
const vec = arrow.vectorFromArray([date])
console.log(vec.toArray())
console.log(vec.toJSON())
console.log(vec.type)
console.log(vec.get(0))
console.log("=> vec2")
const vec2 = arrow.vectorFromArray([date], new arrow.DateMillisecond)
console.log(vec2.toArray())
console.log(vec.toJSON())
console.log(vec2.type)
console.log(vec2.get(0))
console.log("=> table")
const table = arrow.tableFromJSON([{ date }])
console.log(table.toArray())
console.log(table.schema.fields[0].type)
console.log(table.getChildAt(0)?.get(0))
console.log("=> table2")
const table2 = arrow.tableFromIPC(arrow.tableToIPC(table));
console.log(table2.toArray())
console.log(table2.schema.fields[0].type)
console.log(table2.getChildAt(0)?.get(0))
console.log("=> table3")
const table3 = new arrow.Table({ dates: vec2 })
console.log(table3.toArray())
console.log(table3.schema.fields[0].type)
console.log(table3.getChildAt(0)?.get(0))
```
```
=> table
[
{"date": Wed Mar 29 2023 08:34:56 GMT-0400 (Eastern Daylight Time)}
]
TimestampMillisecond {
typeId: 10,
unit: 1,
timezone: undefined,
toString: [Function: toString],
ArrayType: [class Int32Array],
[Symbol(Symbol.toStringTag)]: "Timestamp",
children: null,
OffsetArrayType: [class Int32Array],
}
2023-03-29T12:34:56.000Z
=> table2
[
{"date": Wed Mar 29 2023 08:34:56 GMT-0400 (Eastern Daylight Time)}
]
Timestamp_ {
typeId: 10,
unit: 1,
timezone: null,
toString: [Function: toString],
ArrayType: [class Int32Array],
children: null,
OffsetArrayType: [class Int32Array],
}
2023-03-29T12:34:56.000Z
=> table3
[
{"dates": Wed Mar 29 2023 08:34:56 GMT-0400 (Eastern Daylight Time)}
]
DateMillisecond {
typeId: 8,
unit: 1,
toString: [Function: toString],
ArrayType: [class Int32Array],
[Symbol(Symbol.toStringTag)]: "Date",
children: null,
OffsetArrayType: [class Int32Array],
}
2023-03-29T12:34:56.000Z
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40891: [JS] Store Dates as TimestampMillisecond [arrow]
Posted by "domoritz (via GitHub)" <gi...@apache.org>.
domoritz merged PR #40892:
URL: https://github.com/apache/arrow/pull/40892
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40891: [JS] Store Dates as TimestampMillisecond [arrow]
Posted by "trxcllnt (via GitHub)" <gi...@apache.org>.
trxcllnt commented on code in PR #40892:
URL: https://github.com/apache/arrow/pull/40892#discussion_r1546900725
##########
js/src/type.ts:
##########
@@ -406,7 +406,7 @@ type Timestamps = Type.Timestamp | Type.TimestampSecond | Type.TimestampMillisec
/** @ignore */
interface Timestamp_<T extends Timestamps = Timestamps> extends DataType<T> {
TArray: Int32Array;
- TValue: number;
+ TValue: number | Date;
Review Comment:
Can't the `TValue` type be overridden just for the `TimestampMillisecond` class below?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40891: [JS] Store Dates as TimestampMillisecond [arrow]
Posted by "domoritz (via GitHub)" <gi...@apache.org>.
domoritz commented on PR #40892:
URL: https://github.com/apache/arrow/pull/40892#issuecomment-2030786983
We do construct dates for DateMillisecond already so I figured it would make sense to do the same for TimestampMillisecond.
> IIRC constructing date instances is relatively expensive esp. if using custom locales, so users should opt-in to that behavior if they need it?
I did not measure it but wit surprises me a bit since I expected it to just be a lightweight wrapper around timestamps. I'd be okay with just retuning numbers to be fast but also to introduce an iterator for vectors that returns convenient types in JS that may be slower but easier to work with (in a follow up PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40891: [JS] Store Dates as TimestampMillisecond [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #40892:
URL: https://github.com/apache/arrow/pull/40892#issuecomment-2037273587
After merging your PR, Conbench analyzed the 7 benchmarking runs that have been run so far on merge-commit 2caec860894945e8bfe5b557c825ba962a6a16bd.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/23443942870) has more details.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40891: [JS] Store Dates as TimestampMillisecond [arrow]
Posted by "domoritz (via GitHub)" <gi...@apache.org>.
domoritz commented on code in PR #40892:
URL: https://github.com/apache/arrow/pull/40892#discussion_r1546990776
##########
js/src/type.ts:
##########
@@ -406,7 +406,7 @@ type Timestamps = Type.Timestamp | Type.TimestampSecond | Type.TimestampMillisec
/** @ignore */
interface Timestamp_<T extends Timestamps = Timestamps> extends DataType<T> {
TArray: Int32Array;
- TValue: number;
+ TValue: number | Date;
Review Comment:
Reverted to number again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org