[GitHub] [arrow] github-actions[bot] commented on pull request #9993: ARROW-12317: [Rust] JSON writer support for time and date
github-actions[bot] commented on pull request #9993: URL: https://github.com/apache/arrow/pull/9993#issuecomment-817501699 https://issues.apache.org/jira/browse/ARROW-12317 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] houqp commented on pull request #9993: ARROW-12317: [Rust] JSON writer support for time and date
houqp commented on pull request #9993: URL: https://github.com/apache/arrow/pull/9993#issuecomment-817501688 cc @alamb -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] houqp opened a new pull request #9993: ARROW-12317: [Rust] JSON writer support for time and date
houqp opened a new pull request #9993: URL: https://github.com/apache/arrow/pull/9993 This PR adds support for Time and Date date types in Rust arrow JSON writer module. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] domoritz opened a new pull request #9992: ARROW-12269: [JS] Move to aslant
domoritz opened a new pull request #9992: URL: https://github.com/apache/arrow/pull/9992 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9992: ARROW-12269: [JS] Move to aslant
github-actions[bot] commented on pull request #9992: URL: https://github.com/apache/arrow/pull/9992#issuecomment-817468361 https://issues.apache.org/jira/browse/ARROW-12269 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] mathyingzhou edited a comment on pull request #8648: ARROW-7906: [C++] [Python] Add ORC write support
mathyingzhou edited a comment on pull request #8648: URL: https://github.com/apache/arrow/pull/8648#issuecomment-817451185 @pitrou I completely revamped adapter_util.cc and switched to arrow::ArrayDataVisitor. That is, all comments have been addressed. Please review. By the way I have nothing to do with that Flight error so it shouldn’t prevent a merge. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] msathis commented on pull request #9987: ARROW-12332: [Rust] [Ballista] Add simple api server in scheduler
msathis commented on pull request #9987: URL: https://github.com/apache/arrow/pull/9987#issuecomment-817460796 ``` curl --request GET \ --url http://localhost:50050/executors \ --header 'Accept: application/json' ``` @andygrove The above curl should work. Added small documentation to the PR as well. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] emkornfield commented on pull request #9986: MINOR: [Rust] [CI] Tag Ballista PRs
emkornfield commented on pull request #9986: URL: https://github.com/apache/arrow/pull/9986#issuecomment-817456063 @andygrove were you synced to the latest version, i tried to update the merge tool to also work with minor PRs. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] emkornfield commented on pull request #7214: ARROW-8842: [Java] fix ListVector's setValueCount to set inner vector's value count correctly
emkornfield commented on pull request #7214: URL: https://github.com/apache/arrow/pull/7214#issuecomment-817454551 I don't know why @pprudhvi closed it. There seemed to be a stall in the coversation with @jacques-n if you would like to resurrect it please go ahead. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] mathyingzhou commented on pull request #8648: ARROW-7906: [C++] [Python] Add ORC write support
mathyingzhou commented on pull request #8648: URL: https://github.com/apache/arrow/pull/8648#issuecomment-817451185 @pitrou I completely revamped adapter_util.cc and switched to arrow::ArrayDataVisitor. That is, all comments have been addressed. Please 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] liyafan82 commented on pull request #9964: ARROW-12310: [Java] ValueVector#getObject should support covariance for complex types
liyafan82 commented on pull request #9964: URL: https://github.com/apache/arrow/pull/9964#issuecomment-817430701 > Looks good! > > IMHO, does this PR change the signature of API a bit? Hopefully, the change does not affect existing code practically. @kiszk Thanks for your feedback. Hopefully, it should not affect existing code, as the new return type `List/Map` is a sub-type of the original return type `Object`. If we change the return type in the other direction, client code can be affected. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] andygrove commented on pull request #9987: ARROW-12332: [Rust] [Ballista] Add simple api server in scheduler
andygrove commented on pull request #9987: URL: https://github.com/apache/arrow/pull/9987#issuecomment-817401449 Hi @msathis and thanks for creating the PR. I pulled your branch and tried testing this locally but couldn't get it to work. I'm probably doing something wrong. It would be good if you could add some documentation as part of this PR. Perhaps provide a `curl` command for hitting this API? -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] codecov-io commented on pull request #9991: ARROW-12335: [Rust] [Ballista] Bump DataFusion version [WIP]
codecov-io commented on pull request #9991: URL: https://github.com/apache/arrow/pull/9991#issuecomment-817384523 # [Codecov](https://codecov.io/gh/apache/arrow/pull/9991?src=pr=h1) Report > Merging [#9991](https://codecov.io/gh/apache/arrow/pull/9991?src=pr=desc) (a96fd26) into [master](https://codecov.io/gh/apache/arrow/commit/13c334e976f09d4d896c26d4b5f470e36a46572b?el=desc) (13c334e) will **decrease** coverage by `0.00%`. > The diff coverage is `0.00%`. [](https://codecov.io/gh/apache/arrow/pull/9991?src=pr=tree) ```diff @@Coverage Diff @@ ## master#9991 +/- ## == - Coverage 78.69% 78.68% -0.01% == Files 285 285 Lines 6390063909 +9 == + Hits5028450285 +1 - Misses 1361613624 +8 ``` | [Impacted Files](https://codecov.io/gh/apache/arrow/pull/9991?src=pr=tree) | Coverage Δ | | |---|---|---| | [rust/ballista/rust/client/src/context.rs](https://codecov.io/gh/apache/arrow/pull/9991/diff?src=pr=tree#diff-cnVzdC9iYWxsaXN0YS9ydXN0L2NsaWVudC9zcmMvY29udGV4dC5ycw==) | `0.00% <0.00%> (ø)` | | | [rust/ballista/rust/core/src/datasource.rs](https://codecov.io/gh/apache/arrow/pull/9991/diff?src=pr=tree#diff-cnVzdC9iYWxsaXN0YS9ydXN0L2NvcmUvc3JjL2RhdGFzb3VyY2UucnM=) | `0.00% <ø> (ø)` | | | [...sta/rust/core/src/serde/logical\_plan/from\_proto.rs](https://codecov.io/gh/apache/arrow/pull/9991/diff?src=pr=tree#diff-cnVzdC9iYWxsaXN0YS9ydXN0L2NvcmUvc3JjL3NlcmRlL2xvZ2ljYWxfcGxhbi9mcm9tX3Byb3RvLnJz) | `0.00% <0.00%> (ø)` | | | [...lista/rust/core/src/serde/logical\_plan/to\_proto.rs](https://codecov.io/gh/apache/arrow/pull/9991/diff?src=pr=tree#diff-cnVzdC9iYWxsaXN0YS9ydXN0L2NvcmUvc3JjL3NlcmRlL2xvZ2ljYWxfcGxhbi90b19wcm90by5ycw==) | `0.00% <ø> (ø)` | | | [...ta/rust/core/src/serde/physical\_plan/from\_proto.rs](https://codecov.io/gh/apache/arrow/pull/9991/diff?src=pr=tree#diff-cnVzdC9iYWxsaXN0YS9ydXN0L2NvcmUvc3JjL3NlcmRlL3BoeXNpY2FsX3BsYW4vZnJvbV9wcm90by5ycw==) | `0.00% <0.00%> (ø)` | | | [rust/ballista/rust/scheduler/src/lib.rs](https://codecov.io/gh/apache/arrow/pull/9991/diff?src=pr=tree#diff-cnVzdC9iYWxsaXN0YS9ydXN0L3NjaGVkdWxlci9zcmMvbGliLnJz) | `0.00% <0.00%> (ø)` | | | [rust/parquet/src/encodings/encoding.rs](https://codecov.io/gh/apache/arrow/pull/9991/diff?src=pr=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9lbmNvZGluZ3MvZW5jb2RpbmcucnM=) | `95.05% <0.00%> (+0.19%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow/pull/9991?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/arrow/pull/9991?src=pr=footer). Last update [f1f4f2b...a96fd26](https://codecov.io/gh/apache/arrow/pull/9991?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] andygrove closed pull request #9990: ARROW-12313: [Rust] [Ballista] Update benchmark docs for Ballista
andygrove closed pull request #9990: URL: https://github.com/apache/arrow/pull/9990 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9991: ARROW-12335: [Rust] [Ballista] Bump DataFusion version [WIP]
github-actions[bot] commented on pull request #9991: URL: https://github.com/apache/arrow/pull/9991#issuecomment-817382555 https://issues.apache.org/jira/browse/ARROW-12335 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] andygrove opened a new pull request #9991: ARROW-12335: [Rust] [Ballista] Bump DataFusion version
andygrove opened a new pull request #9991: URL: https://github.com/apache/arrow/pull/9991 Tests fail. Ballista does not handle the new `RepartitionExec` that DataFusion is inserting. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9990: ARROW-12313: [Rust] [Ballista] Update benchmark docs for Ballista
github-actions[bot] commented on pull request #9990: URL: https://github.com/apache/arrow/pull/9990#issuecomment-817378904 https://issues.apache.org/jira/browse/ARROW-12313 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] andygrove opened a new pull request #9990: ARROW-12313: [Rust] [Ballista] Update benchmark docs for Ballista
andygrove opened a new pull request #9990: URL: https://github.com/apache/arrow/pull/9990 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] andygrove merged pull request #9986: MINOR: [Rust] [CI] Tag Ballista PRs
andygrove merged pull request #9986: URL: https://github.com/apache/arrow/pull/9986 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] kou commented on pull request #9989: MINOR: Fix link to definition of minor
kou commented on pull request #9989: URL: https://github.com/apache/arrow/pull/9989#issuecomment-817372936 Good catch! -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] kou merged pull request #9989: MINOR: Fix link to definition of minor
kou merged pull request #9989: URL: https://github.com/apache/arrow/pull/9989 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] kou closed pull request #9983: ARROW-12274: [JS] Document how to run tests without building bundles
kou closed pull request #9983: URL: https://github.com/apache/arrow/pull/9983 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] kou commented on pull request #9986: MINOR: [Rust] [CI] Tag Ballista PRs
kou commented on pull request #9986: URL: https://github.com/apache/arrow/pull/9986#issuecomment-817369647 We can use the "Squash and merge" button in 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] domoritz commented on a change in pull request #9988: ARROW-12333: [JS] Remove jest-environment-node-debug and do not emit from typescript by default
domoritz commented on a change in pull request #9988:
URL: https://github.com/apache/arrow/pull/9988#discussion_r611241493
##
File path: js/tsconfig.json
##
@@ -6,6 +6,7 @@
},
"compilerOptions": {
"target": "ESNEXT",
-"module": "commonjs"
+"module": "commonjs",
+"noEmit": true
Review comment:
Now we can run `yarn tsc` to check types without accidentally writing a
bunch of files.
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [arrow] domoritz commented on a change in pull request #9988: ARROW-12333: [JS] Remove jest-environment-node-debug and do not emit from typescript by default
domoritz commented on a change in pull request #9988:
URL: https://github.com/apache/arrow/pull/9988#discussion_r611241493
##
File path: js/tsconfig.json
##
@@ -6,6 +6,7 @@
},
"compilerOptions": {
"target": "ESNEXT",
-"module": "commonjs"
+"module": "commonjs",
+"noEmit": true
Review comment:
Now we can run yarn tsc to check types without accidentally writing a
bunch of files.
##
File path: js/package.json
##
@@ -86,7 +86,6 @@
"gulp-typescript": "5.0.1",
"ix": "2.5.3",
"jest": "26.3.0",
-"jest-environment-node-debug": "2.0.0",
Review comment:
I didn't see it being used anywhere.
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9988: ARROW-12333: [JS] Remove jest-environment-node-debug and do not emit from typescript by default
github-actions[bot] commented on pull request #9988: URL: https://github.com/apache/arrow/pull/9988#issuecomment-817367323 https://issues.apache.org/jira/browse/ARROW-12333 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] domoritz opened a new pull request #9989: MINOR: Fix link to definition of minor
domoritz opened a new pull request #9989: URL: https://github.com/apache/arrow/pull/9989 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] msathis edited a comment on pull request #9987: ARROW-12332: [Rust] Add simple api server in scheduler
msathis edited a comment on pull request #9987: URL: https://github.com/apache/arrow/pull/9987#issuecomment-817354924 https://user-images.githubusercontent.com/2977899/114317329-2a421080-9b25-11eb-90ae-4161476b3d8c.png;> @andygrove Please review when you find 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9988: Remove jest-environment-node-debug and do not emit from typescript by default
github-actions[bot] commented on pull request #9988:
URL: https://github.com/apache/arrow/pull/9988#issuecomment-817366444
Thanks for opening a pull request!
If this is not a [minor
PR](https://github.com/apache/arrow/blob/master/.github/CONTRIBUTING.md#Minor-Fixes).
Could you open an issue for this pull request on JIRA?
https://issues.apache.org/jira/browse/ARROW
Opening JIRAs ahead of time contributes to the
[Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.)
of the Apache Arrow project.
Then could you also rename pull request title in the following format?
ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}
or
MINOR: [${COMPONENT}] ${SUMMARY}
See also:
* [Other pull requests](https://github.com/apache/arrow/pulls/)
* [Contribution Guidelines - How to contribute
patches](https://arrow.apache.org/docs/developers/contributing.html#how-to-contribute-patches)
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [arrow] domoritz opened a new pull request #9988: Remove jest-environment-node-debug and do not emit from typescript by default
domoritz opened a new pull request #9988: URL: https://github.com/apache/arrow/pull/9988 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] domoritz commented on pull request #9961: ARROW-12309: [JS] Make es2015 bundles the default
domoritz commented on pull request #9961: URL: https://github.com/apache/arrow/pull/9961#issuecomment-817366232 I didn't mean not having bundles. I mean that the bundle will default to es2015. The es5 bundle wouldn't be as optimal anymore but the es5 bundles don't seem to get much traffic anyway: https://www.npmjs.com/search?q=apache-arrow%2Fes5. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] trxcllnt commented on pull request #9961: ARROW-12309: [JS] Make es2015 bundles the default
trxcllnt commented on pull request #9961: URL: https://github.com/apache/arrow/pull/9961#issuecomment-817365840 @domoritz people definitely use it in jsfiddles/notebooks 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] kou closed pull request #9938: ARROW-12281: [JS] Remove shx, trash, and rimraf and update learna for yarn
kou closed pull request #9938: URL: https://github.com/apache/arrow/pull/9938 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] msathis commented on pull request #9987: ARROW-12332: [Rust] Add simple api server in scheduler
msathis commented on pull request #9987: URL: https://github.com/apache/arrow/pull/9987#issuecomment-817354924 https://user-images.githubusercontent.com/2977899/114317329-2a421080-9b25-11eb-90ae-4161476b3d8c.png;> -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9987: ARROW-12332: [Rust] Add simple api server in scheduler
github-actions[bot] commented on pull request #9987: URL: https://github.com/apache/arrow/pull/9987#issuecomment-817354732 https://issues.apache.org/jira/browse/ARROW-12332 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9987: Add simple api server in scheduler
github-actions[bot] commented on pull request #9987:
URL: https://github.com/apache/arrow/pull/9987#issuecomment-817354014
Thanks for opening a pull request!
If this is not a [minor
PR](https://github.com/apache/arrow/blob/master/.github/CONTRIBUTING.md#Minor-Fixes).
Could you open an issue for this pull request on JIRA?
https://issues.apache.org/jira/browse/ARROW
Opening JIRAs ahead of time contributes to the
[Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.)
of the Apache Arrow project.
Then could you also rename pull request title in the following format?
ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}
or
MINOR: [${COMPONENT}] ${SUMMARY}
See also:
* [Other pull requests](https://github.com/apache/arrow/pulls/)
* [Contribution Guidelines - How to contribute
patches](https://arrow.apache.org/docs/developers/contributing.html#how-to-contribute-patches)
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [arrow] msathis opened a new pull request #9987: Add simple api server in scheduler
msathis opened a new pull request #9987: URL: https://github.com/apache/arrow/pull/9987 Implements GET /executors. We can additional endpoints going forward. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] trxcllnt commented on a change in pull request #9962: ARROW-12303: [JS] Use iterator instead of yield
trxcllnt commented on a change in pull request #9962:
URL: https://github.com/apache/arrow/pull/9962#discussion_r611226795
##
File path: js/src/util/bit.ts
##
@@ -64,16 +64,51 @@ export function packBools(values: Iterable) {
}
/** @ignore */
-export function* iterateBits(bytes: Uint8Array, begin: number, length:
number, context: any,
-get: (context: any, index: number, byte:
number, bit: number) => T) {
-let bit = begin % 8;
-let byteIndex = begin >> 3;
-let index = 0, remaining = length;
-for (; remaining > 0; bit = 0) {
-let byte = bytes[byteIndex++];
-do {
-yield get(context, index++, byte, bit);
-} while (--remaining > 0 && ++bit < 8);
+export class BitIterator implements IterableIterator {
+bit: number;
+byte: number;
+byteIndex: number;
+index: number;
+
+constructor(
+private bytes: Uint8Array,
+begin: number,
+private length: number,
+private context: any,
+private get: (context: any, index: number, byte: number, bit: number)
=> T
+) {
+this.bit = begin % 8;
+this.byteIndex = begin >> 3;
+this.byte = bytes[this.byteIndex];
+this.index = 0;
+}
+
+next(): IteratorResult {
+if (this.index >= this.length) {
+return {
+done: true,
+value: null
+};
+}
+
+if (this.bit < 8) {
+return {
+value: this.get(this.context, this.index++, this.byte,
this.bit++)
+};
+}
+
+this.byteIndex++;
+this.bit = 0;
+
+if (this.byteIndex < this.bytes.length) {
+this.byte = this.bytes[this.byteIndex];
+}
+
+return this.next();
+}
Review comment:
Can we do this without recursion?
```suggestion
next(): IteratorResult {
if (this.index < this.length) {
if (this.bit === 8) {
this.bit = 0;
this.byte = this.bytes[this.byteIndex++];
}
return {
done: false,
value: this.get(this.context, this.index++, this.byte,
this.bit++)
};
}
return { done: true, value: null };
}
```
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [arrow] trxcllnt commented on a change in pull request #9962: ARROW-12303: [JS] Use iterator instead of yield
trxcllnt commented on a change in pull request #9962:
URL: https://github.com/apache/arrow/pull/9962#discussion_r611226724
##
File path: js/src/util/bit.ts
##
@@ -64,16 +64,51 @@ export function packBools(values: Iterable) {
}
/** @ignore */
-export function* iterateBits(bytes: Uint8Array, begin: number, length:
number, context: any,
-get: (context: any, index: number, byte:
number, bit: number) => T) {
-let bit = begin % 8;
-let byteIndex = begin >> 3;
-let index = 0, remaining = length;
-for (; remaining > 0; bit = 0) {
-let byte = bytes[byteIndex++];
-do {
-yield get(context, index++, byte, bit);
-} while (--remaining > 0 && ++bit < 8);
+export class BitIterator implements IterableIterator {
+bit: number;
+byte: number;
+byteIndex: number;
+index: number;
+
+constructor(
+private bytes: Uint8Array,
+begin: number,
+private length: number,
+private context: any,
+private get: (context: any, index: number, byte: number, bit: number)
=> T
+) {
+this.bit = begin % 8;
+this.byteIndex = begin >> 3;
+this.byte = bytes[this.byteIndex];
Review comment:
```suggestion
this.byte = bytes[this.byteIndex++];
```
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [arrow] houqp commented on pull request #9968: ARROW-12267: [Rust] Implement support for timestamps in JSON writer
houqp commented on pull request #9968: URL: https://github.com/apache/arrow/pull/9968#issuecomment-817347780 The caller would need to obtain the response schema first, in most cases, the schema should be static. In case of API responses, it would be part of the API spec. I would expect most of the users to use the string encoded format and only resort to numbers for performance critical use-cases. For example, in Airflow, by changing the column type from string encoded date time to timestamp numbers, we reduced the frontend load time by almost 90% :) -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] domoritz commented on pull request #9961: ARROW-12309: [JS] Make es2015 bundles the default
domoritz commented on pull request #9961: URL: https://github.com/apache/arrow/pull/9961#issuecomment-817342595 I wonder, would it make sense to remove closure compiler if we only use it for es5 and es5 isn't the default bundle anymore? Is the benefit for users big enough to warrant the overhead for devs? -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] andygrove commented on pull request #9986: MINOR: [Rust] [CI] Tag Ballista PRs
andygrove commented on pull request #9986: URL: https://github.com/apache/arrow/pull/9986#issuecomment-817337302 How do we merge PRs that have no JIRA? The `merge_arrow_pr` script fails. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] andygrove commented on pull request #9986: [TRIVIAL] Tag Ballista PRs
andygrove commented on pull request #9986: URL: https://github.com/apache/arrow/pull/9986#issuecomment-817332981 @jorgecarleitao @alamb Is this all that is required? -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] andygrove opened a new pull request #9986: [TRIVIAL] Tag Ballista PRs
andygrove opened a new pull request #9986: URL: https://github.com/apache/arrow/pull/9986 I did not create a JIRA for this one. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] andygrove closed pull request #9979: ARROW-12251: [Rust] Add Ballista to CI
andygrove closed pull request #9979: URL: https://github.com/apache/arrow/pull/9979 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] andygrove commented on pull request #9979: ARROW-12251: [Rust] Add Ballista to CI
andygrove commented on pull request #9979: URL: https://github.com/apache/arrow/pull/9979#issuecomment-817332160 I confirmed that CI ran the build and tests for Ballista so will go ahead and merge this. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] andygrove commented on pull request #9979: ARROW-12251: [Rust] Add Ballista to CI
andygrove commented on pull request #9979: URL: https://github.com/apache/arrow/pull/9979#issuecomment-817330050 I modified this PR to build without default features, just so we can enable CI quickly. I filed https://issues.apache.org/jira/browse/ARROW-12331 for the follow-up work for snmalloc. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #9975: ARROW-12325: [C++] [CI] Nightly gandiva build failing due to failure of compiler to move return value
nealrichardson commented on pull request #9975: URL: https://github.com/apache/arrow/pull/9975#issuecomment-817325048 > I'm open to suggestions Maybe add a comment to this effect so that whenever we remove support for gcc < 4.9, we can remove this? Or could you even use an `#ifdef` or something to only do this on gcc < 4.9? -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] domoritz commented on a change in pull request #9962: ARROW-12303: [JS] Use iterator instead of yield
domoritz commented on a change in pull request #9962:
URL: https://github.com/apache/arrow/pull/9962#discussion_r611202891
##
File path: js/src/util/bit.ts
##
@@ -64,16 +64,52 @@ export function packBools(values: Iterable) {
}
/** @ignore */
-export function* iterateBits(bytes: Uint8Array, begin: number, length:
number, context: any,
-get: (context: any, index: number, byte:
number, bit: number) => T) {
-let bit = begin % 8;
-let byteIndex = begin >> 3;
-let index = 0, remaining = length;
-for (; remaining > 0; bit = 0) {
-let byte = bytes[byteIndex++];
-do {
-yield get(context, index++, byte, bit);
-} while (--remaining > 0 && ++bit < 8);
+// @ts-ignore
Review comment:
```suggestion
```
Ups, left some debugging code in 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [arrow] kiszk opened a new pull request #9985: ARROW-12330: [Developer] Restore values at counters column of Archery benchmark
kiszk opened a new pull request #9985:
URL: https://github.com/apache/arrow/pull/9985
This PR restores values at `counters` column of Archery benchmark. #9140
always suppressed values at `counters` column regardless `--no-counter`. In
addition, this PR stores `counters` value into json file.
Before
```
% archery benchmark diff --benchmark-filter="SetBitsTo" --output=head2.json
HEAD HEAD~1
...
---
Benchmark Time CPU Iterations UserCounters...
---
SetBitsTo/28.15 ns 8.15 ns 81991087
bytes_per_second=234.044M/s
SetBitsTo/16 7.78 ns 7.78 ns 89928878
bytes_per_second=1.91429G/s
SetBitsTo/1024 13.9 ns 13.9 ns 50372172
bytes_per_second=68.6182G/s
SetBitsTo/131072 3508 ns 3508 ns 199335
bytes_per_second=34.7944G/s
--
Non-regressions: (4)
--
benchmark baselinecontender change % counters
SetBitsTo/161.877 GiB/sec1.914 GiB/sec 1.975 {}
SetBitsTo/2 230.566 MiB/sec 234.044 MiB/sec 1.509 {}
SetBitsTo/131072 34.722 GiB/sec 34.794 GiB/sec 0.207 {}
SetBitsTo/1024 68.593 GiB/sec 68.618 GiB/sec 0.037 {}
```
After
```
---
Benchmark Time CPU Iterations UserCounters...
---
SetBitsTo/28.39 ns 8.39 ns 81980047
bytes_per_second=227.438M/s
SetBitsTo/16 7.88 ns 7.88 ns 84936624
bytes_per_second=1.89105G/s
SetBitsTo/1024 13.9 ns 13.9 ns 50376587
bytes_per_second=68.6064G/s
SetBitsTo/131072 3513 ns 3513 ns 200598
bytes_per_second=34.7447G/s
Non-regressions: (4)
benchmark baselinecontender change %
counters
SetBitsTo/2 227.438 MiB/sec 235.984 MiB/sec 3.757
{'run_name': 'SetBitsTo/2', 'repetitions': 0, 'repetition_index': 0, 'threads':
1, 'iterations': 81980047}
SetBitsTo/161.891 GiB/sec1.913 GiB/sec 1.137
{'run_name': 'SetBitsTo/16', 'repetitions': 0, 'repetition_index': 0,
'threads': 1, 'iterations': 84936624}
SetBitsTo/131072 34.745 GiB/sec 34.771 GiB/sec 0.075 {'run_name':
'SetBitsTo/131072', 'repetitions': 0, 'repetition_index': 0, 'threads': 1,
'iterations': 200598}
SetBitsTo/1024 68.606 GiB/sec 68.624 GiB/sec 0.026 {'run_name':
'SetBitsTo/1024', 'repetitions': 0, 'repetition_index': 0, 'threads': 1,
'iterations': 50376587}
```
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9985: ARROW-12330: [Developer] Restore values at counters column of Archery benchmark
github-actions[bot] commented on pull request #9985: URL: https://github.com/apache/arrow/pull/9985#issuecomment-817322813 https://issues.apache.org/jira/browse/ARROW-12330 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] Dandandan edited a comment on pull request #9970: ARROW-12277: [Rust][DataFusion] Implement Sum/Count/Min/Max aggregates for Timestamp(_,_)
Dandandan edited a comment on pull request #9970: URL: https://github.com/apache/arrow/pull/9970#issuecomment-817317167 > > What are the exact use cases for summing timestamps? When does it make sense? > > @Dandandan that is an excellent question. I will freely admit I was just heads down trying to get all the aggregates working rather than thinking if I _should_ be working. I can't think of any time that `sum(timestamp)` really makes sense to be honest. > > Do you think I should remove support for summing timestamps? > > As for AVG, I added a note to https://issues.apache.org/jira/browse/ARROW-12318 with your observation. 樂 good question At least would be best to remove support for sum, as we don't have an absolute zero for timestamps. For average, I think it may make some more sense, but I think the use case seems limited 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] Dandandan commented on pull request #9970: ARROW-12277: [Rust][DataFusion] Implement Sum/Count/Min/Max aggregates for Timestamp(_,_)
Dandandan commented on pull request #9970: URL: https://github.com/apache/arrow/pull/9970#issuecomment-817317167 > > What are the exact use cases for summing timestamps? When does it make sense? > > @Dandandan that is an excellent question. I will freely admit I was just heads down trying to get all the aggregates working rather than thinking if I _should_ be working. I can't think of any time that `sum(timestamp)` really makes sense to be honest. > > Do you think I should remove support for summing timestamps? > > As for AVG, I added a note to https://issues.apache.org/jira/browse/ARROW-12318 with your observation. 樂 good question At least would be best to remove support forsum, as we don't have an absolute zero for timestamps. For average, I think it may make some more sense, but I think the use case seems limited 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] Dandandan commented on pull request #9979: ARROW-12251: [Rust] Add Ballista to CI
Dandandan commented on pull request #9979: URL: https://github.com/apache/arrow/pull/9979#issuecomment-817315885 Thanks @mjp41 I think that's a great option. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] roee88 commented on a change in pull request #9631: ARROW-11644: [Python][Parquet] Low-level Parquet decryption in Python
roee88 commented on a change in pull request #9631:
URL: https://github.com/apache/arrow/pull/9631#discussion_r611172212
##
File path: cpp/src/parquet/encryption/encryption.h
##
@@ -70,6 +70,26 @@ class PARQUET_EXPORT StringKeyIdRetriever : public
DecryptionKeyRetriever {
std::map key_map_;
};
+// Function variant of DecryptionKeyRetriever, taking a state object.
Review comment:
Not strictly related to the above discussion, but usually these classes
are implemented in
https://github.com/apache/arrow/tree/master/cpp/src/arrow/python as far as I
can tell.
In
https://docs.google.com/document/d/1i1M5f5azLEmASj9XQZ_aQLl5Fr5F0CvnyPPVu1xaD9U/edit
@emkornfield suggested to look at PyFileSystem as an example and I suggest to
follow the same pattern in terms of implementation (vtable,
ARROW_PYTHON_EXPORT, and anything else).
> It's not possible to subclass a C++ class in Cython
To be fair to Cython, it's possible (see an
[example](https://gist.github.com/roee88/49d22e28cfd95dbabde2813e067594aa)).
However, it's not well documented and might be less efficient w.r.t. GIL so the
vtable approach used in the arrow codebase is probably better.
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [arrow] roee88 commented on a change in pull request #9631: ARROW-11644: [Python][Parquet] Low-level Parquet decryption in Python
roee88 commented on a change in pull request #9631:
URL: https://github.com/apache/arrow/pull/9631#discussion_r611172212
##
File path: cpp/src/parquet/encryption/encryption.h
##
@@ -70,6 +70,26 @@ class PARQUET_EXPORT StringKeyIdRetriever : public
DecryptionKeyRetriever {
std::map key_map_;
};
+// Function variant of DecryptionKeyRetriever, taking a state object.
Review comment:
Not strictly related to the above discussion, but usually these classes
are implemented in
https://github.com/apache/arrow/tree/master/cpp/src/arrow/python as far as I
can tell.
In
https://docs.google.com/document/d/1i1M5f5azLEmASj9XQZ_aQLl5Fr5F0CvnyPPVu1xaD9U/edit
@emkornfield suggested to look at PyFileSystem as an example and I suggest to
follow the same pattern in terms of implementation (vtable,
ARROW_PYTHON_EXPORT, and anything else).
> It's not possible to subclass a C++ class in Cython
To be fail to Cython, it's possible (see an
[example](https://gist.github.com/roee88/49d22e28cfd95dbabde2813e067594aa)).
However, it's not well documented and might be less efficient w.r.t. GIL so the
vtable approach used in the arrow codebase is probably better.
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [arrow] alamb commented on pull request #9970: ARROW-12277: [Rust][DataFusion] Implement Sum/Count/Min/Max aggregates for Timestamp(_,_)
alamb commented on pull request #9970: URL: https://github.com/apache/arrow/pull/9970#issuecomment-817279890 > What are the exact use cases for summing timestamps? When does it make sense? @Dandandan that is an excellent question. I will freely admit I was just heads down trying to get all the aggregates working rather than thinking if I *should* be working. I can't think of any time that `sum(timestamp)` really makes sense to be honest. Do you think I should remove support for summing timestamps? As for AVG, I added a note to https://issues.apache.org/jira/browse/ARROW-12318 with your observation. 樂 good question -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] alamb closed pull request #9968: ARROW-12267: [Rust] Implement support for timestamps in JSON writer
alamb closed pull request #9968: URL: https://github.com/apache/arrow/pull/9968 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] alamb commented on pull request #9968: ARROW-12267: [Rust] Implement support for timestamps in JSON writer
alamb commented on pull request #9968:
URL: https://github.com/apache/arrow/pull/9968#issuecomment-817277440
> I am thinking of adding an option to return these type of values as
numbers instead of human readable strings for better ser/de performance, what
do you think about that?
@houqp I think an option for such behavior is a reasonable idea.
One question I have is if dates / timestamps / etc are produced as numbers,
how would the other side know what type to use?
Like if we made JSON like this:
```
{
"name": "foo",
"time": 1234456788
}
```
Is `time` a `Timestamp`? A `Time`? a `u64`? something else?
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [arrow] mjp41 commented on pull request #9979: ARROW-12251: [Rust] Add Ballista to CI
mjp41 commented on pull request #9979: URL: https://github.com/apache/arrow/pull/9979#issuecomment-817267751 This came up for another project, and they added support for building snmalloc-[rs/sys] without cmake: > build_cc: use of cc crate instead of cmake (cmake still default) as builder (more platform agnostic) > https://github.com/SchrodingerZhu/snmalloc-rs/pull/118 Personally, I would prefer snmalloc to be built with cmake, as that is what we test the most. But, if that is not possible for your scenario, this option is 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
