You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "tdcmeehan (via GitHub)" <gi...@apache.org> on 2023/09/01 20:25:27 UTC
[GitHub] [arrow] tdcmeehan opened a new pull request, #37528: GH-37527: [Java] Allow FlightInfo#Schema to be nullable for long-runn…
tdcmeehan opened a new pull request, #37528:
URL: https://github.com/apache/arrow/pull/37528
…ing queries
With #36155, implementations of Flight RPC may not return quickly via a newly added pollFlightInfo function. Sometimes, the system implementing this function may not know the output schema for some time--for example, after a lengthy queue time as elapsed, or after planning.
In proto3, fields may not be present, and it's a coding convention to require them 1. To support upcoming client integration work for pollFlightInfo, the schema field can be made optional so that it's not a requirement to populate the FlightInfo's schema on the first pollFlightInfo request.
We can modify our client code to allow this field to be optional. This is already the case for the Go code.
This changes the Java client code to allow the Schema to be null. `getSchema` methods now return `Optional<Schema>`, which is a backwards incompatible change.
### Rationale for this change
With #36155, implementations of Flight RPC may not return quickly via a newly added pollFlightInfo function. Sometimes, the system implementing this function may not know the output schema for some time--for example, after a lengthy queue time as elapsed, or after planning.
In proto3, fields may not be present, and it's a coding convention to require them 1. To support upcoming client integration work for pollFlightInfo, the schema field can be made optional so that it's not a requirement to populate the FlightInfo's schema on the first pollFlightInfo request.
CC: @lidavidm
### What changes are included in this PR?
This changes the Java client code to allow the Schema to be null. `getSchema` methods now return `Optional<Schema>`, which is a backwards incompatible change.
### Are these changes tested?
Existing tests ensure serialization and deserialization continue to work.
### Are there any user-facing changes?
The `getSchema` methods are now Optional.
<!--
Please uncomment the line below (and provide explanation) if the changes fix either (a) a security vulnerability, (b) a bug that caused incorrect or invalid data to be produced, or (c) a bug that causes a crash (even when the API contract is upheld). We use this to highlight fixes to issues that may affect users without their knowledge. For this reason, fixing bugs that cause errors don't count, since those are usually obvious.
-->
<!-- **This PR contains a "Critical Fix".** -->
--
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
[GitHub] [arrow] lidavidm commented on a diff in pull request #37528: GH-37553: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #37528:
URL: https://github.com/apache/arrow/pull/37528#discussion_r1317726188
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightInfo.java:
##########
@@ -130,6 +131,14 @@ public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoin
option = IpcOption.DEFAULT;
}
+ public Optional<Schema> getSchemaOptional() {
+ return Optional.ofNullable(schema);
+ }
+
+ /**
+ * Returns the schema, or null if no schema is present.
+ * @deprecated Deprecated. Use {@link #getSchemaOptional()} instead.
+ */
public Schema getSchema() {
Review Comment:
Actually add the `@Deprecated` annotation?
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightInfo.java:
##########
@@ -130,6 +131,14 @@ public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoin
option = IpcOption.DEFAULT;
}
+ public Optional<Schema> getSchemaOptional() {
+ return Optional.ofNullable(schema);
+ }
+
+ /**
+ * Returns the schema, or null if no schema is present.
Review Comment:
For maximum compatibility, we could return a schema of no fields if the underlying schema is null, the same way it appears to have done before.
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightProducer.java:
##########
@@ -77,7 +77,9 @@ default PollInfo pollFlightInfo(CallContext context, FlightDescriptor descriptor
*/
default SchemaResult getSchema(CallContext context, FlightDescriptor descriptor) {
FlightInfo info = getFlightInfo(context, descriptor);
- return new SchemaResult(info.getSchema());
+ return new SchemaResult(info
+ .getSchemaOptional()
+ .orElseThrow(() -> new IllegalArgumentException("No schema is present in FlightInfo")));
Review Comment:
It would be better to throw `CallStatus.INVALID_ARGUMENT.withDescription(...).asRuntimeException()`
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightInfo.java:
##########
@@ -93,7 +93,6 @@ public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoin
*/
public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoint> endpoints, long bytes,
long records, boolean ordered, IpcOption option) {
- Objects.requireNonNull(schema);
Objects.requireNonNull(descriptor);
Objects.requireNonNull(endpoints);
MetadataV4UnionChecker.checkForUnion(schema.getFields().iterator(), option.metadataVersion);
Review Comment:
This needs to be made conditional on the schema. (Can we cover this in a unit test?)
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightInfo.java:
##########
@@ -161,7 +163,10 @@ Flight.FlightInfo toProtocol() {
// Encode schema in a Message payload
ByteArrayOutputStream baos = new ByteArrayOutputStream();
try {
- MessageSerializer.serialize(new WriteChannel(Channels.newChannel(baos)), schema, option);
+ MessageSerializer.serialize(
Review Comment:
This doesn't seem resolved? We shouldn't call `setSchema` or run this code in the first place if `schema` is null.
##########
java/vector/src/main/java/org/apache/arrow/vector/ipc/message/MessageSerializer.java:
##########
@@ -183,6 +183,9 @@ public static ByteBuffer serializeMetadata(Schema schema) {
* Returns the serialized flatbuffer bytes of the schema wrapped in a message table.
*/
public static ByteBuffer serializeMetadata(Schema schema, IpcOption writeOption) {
+ if (schema == null) {
+ return ByteBuffer.allocate(0);
+ }
Review Comment:
I think we should adjust callsites instead of implicitly doing something that isn't valid in all contexts (ditto for the two instances below, too)
--
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
[GitHub] [arrow] github-actions[bot] commented on pull request #37528: GH-37527: [Java] Allow FlightInfo#Schema to be nullable for long-runn…
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37528:
URL: https://github.com/apache/arrow/pull/37528#issuecomment-1703284179
:warning: GitHub issue #37527 **has been automatically assigned in GitHub** to PR creator.
--
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
[GitHub] [arrow] lidavidm merged pull request #37528: GH-37553: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #37528:
URL: https://github.com/apache/arrow/pull/37528
--
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
[GitHub] [arrow] tdcmeehan commented on pull request #37528: GH-37553: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "tdcmeehan (via GitHub)" <gi...@apache.org>.
tdcmeehan commented on PR #37528:
URL: https://github.com/apache/arrow/pull/37528#issuecomment-1711761421
Thank you for merging and advice @lidavidm and @kou
--
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
[GitHub] [arrow] kou commented on pull request #37528: GH-37527: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #37528:
URL: https://github.com/apache/arrow/pull/37528#issuecomment-1703331770
It seems that the current implementation already accepts the empty schema message. And an empty schema is used for the case.
https://github.com/apache/arrow/blob/a43206846a02028fc2ac5091c388aa65b07799c1/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightInfo.java#L115-L118
How about passing an empty instead of accepting `null` schema when we can't create a schema for long-running queries?
What is the difference of the above explicit empty schema approach and your suggested implicit empty schema approach? Easy to write?
--
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
[GitHub] [arrow] lidavidm commented on pull request #37528: GH-37553: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #37528:
URL: https://github.com/apache/arrow/pull/37528#issuecomment-1710189602
(I tweaked the PR description to reflect the changes made.)
--
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
[GitHub] [arrow] lidavidm commented on pull request #37528: GH-37527: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #37528:
URL: https://github.com/apache/arrow/pull/37528#issuecomment-1705385844
> (Do we need to update the C++ implementation too?)
Yes, the C++ implementation has the same (mistaken) assumption.
--
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
[GitHub] [arrow] lidavidm commented on a diff in pull request #37528: GH-37553: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #37528:
URL: https://github.com/apache/arrow/pull/37528#discussion_r1318598911
##########
java/vector/src/main/java/org/apache/arrow/vector/ipc/message/MessageSerializer.java:
##########
@@ -212,7 +212,7 @@ public static Schema deserializeSchema(Message schemaMessage) {
public static Schema deserializeSchema(ReadChannel in) throws IOException {
MessageMetadataResult result = readMessage(in);
if (result == null) {
- throw new IOException("Unexpected end of input when reading Schema");
+ return null;
Review Comment:
I think we can revert this?
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightInfo.java:
##########
@@ -130,8 +134,17 @@ public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoin
option = IpcOption.DEFAULT;
}
+ public Optional<Schema> getSchemaOptional() {
+ return Optional.ofNullable(schema);
+ }
+
+ /**
+ * Returns the schema, or null if no schema is present.
Review Comment:
The docstring is now out-of-date.
--
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
[GitHub] [arrow] conbench-apache-arrow[bot] commented on pull request #37528: GH-37553: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #37528:
URL: https://github.com/apache/arrow/pull/37528#issuecomment-1711758531
After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 48cc2abe29fb261317d676090931b245e82f136e.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/16619772235) has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them.
--
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
[GitHub] [arrow] lidavidm commented on pull request #37528: GH-37553: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #37528:
URL: https://github.com/apache/arrow/pull/37528#issuecomment-1705817065
I can likely help for C++, will just take me a while to be able to get around to it
--
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
[GitHub] [arrow] tdcmeehan commented on a diff in pull request #37528: GH-37553: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "tdcmeehan (via GitHub)" <gi...@apache.org>.
tdcmeehan commented on code in PR #37528:
URL: https://github.com/apache/arrow/pull/37528#discussion_r1317476418
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightInfo.java:
##########
@@ -130,8 +132,8 @@ public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoin
option = IpcOption.DEFAULT;
}
- public Schema getSchema() {
- return schema;
+ public Optional<Schema> getSchema() {
Review Comment:
Sure, done!
--
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
[GitHub] [arrow] lidavidm commented on a diff in pull request #37528: GH-37527: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #37528:
URL: https://github.com/apache/arrow/pull/37528#discussion_r1313487235
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightInfo.java:
##########
@@ -130,8 +132,8 @@ public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoin
option = IpcOption.DEFAULT;
}
- public Schema getSchema() {
- return schema;
+ public Optional<Schema> getSchema() {
Review Comment:
Hmm, this is a breaking change. Is it possible to deprecate instead and add a separate getter?
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightInfo.java:
##########
@@ -161,7 +163,10 @@ Flight.FlightInfo toProtocol() {
// Encode schema in a Message payload
ByteArrayOutputStream baos = new ByteArrayOutputStream();
try {
- MessageSerializer.serialize(new WriteChannel(Channels.newChannel(baos)), schema, option);
+ MessageSerializer.serialize(
Review Comment:
We can skip setting the schema entirely if there is no schema. (Otherwise this will serialize to a schema with no fields, instead of no schema.)
--
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
[GitHub] [arrow] tdcmeehan commented on a diff in pull request #37528: GH-37553: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "tdcmeehan (via GitHub)" <gi...@apache.org>.
tdcmeehan commented on code in PR #37528:
URL: https://github.com/apache/arrow/pull/37528#discussion_r1315252400
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightInfo.java:
##########
@@ -130,8 +132,8 @@ public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoin
option = IpcOption.DEFAULT;
}
- public Schema getSchema() {
- return schema;
+ public Optional<Schema> getSchema() {
Review Comment:
We could do that, but do you have any suggestions on the name of the new getter?
--
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
[GitHub] [arrow] tdcmeehan commented on a diff in pull request #37528: GH-37553: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "tdcmeehan (via GitHub)" <gi...@apache.org>.
tdcmeehan commented on code in PR #37528:
URL: https://github.com/apache/arrow/pull/37528#discussion_r1315252198
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightInfo.java:
##########
@@ -161,7 +163,10 @@ Flight.FlightInfo toProtocol() {
// Encode schema in a Message payload
ByteArrayOutputStream baos = new ByteArrayOutputStream();
try {
- MessageSerializer.serialize(new WriteChannel(Channels.newChannel(baos)), schema, option);
+ MessageSerializer.serialize(
Review Comment:
Yes, done.
--
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
[GitHub] [arrow] tdcmeehan commented on a diff in pull request #37528: GH-37553: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "tdcmeehan (via GitHub)" <gi...@apache.org>.
tdcmeehan commented on code in PR #37528:
URL: https://github.com/apache/arrow/pull/37528#discussion_r1318629023
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightInfo.java:
##########
@@ -130,8 +134,17 @@ public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoin
option = IpcOption.DEFAULT;
}
+ public Optional<Schema> getSchemaOptional() {
+ return Optional.ofNullable(schema);
+ }
+
+ /**
+ * Returns the schema, or null if no schema is present.
Review Comment:
Thanks, fixed it.
##########
java/vector/src/main/java/org/apache/arrow/vector/ipc/message/MessageSerializer.java:
##########
@@ -212,7 +212,7 @@ public static Schema deserializeSchema(Message schemaMessage) {
public static Schema deserializeSchema(ReadChannel in) throws IOException {
MessageMetadataResult result = readMessage(in);
if (result == null) {
- throw new IOException("Unexpected end of input when reading Schema");
+ return null;
Review Comment:
You're right, done.
--
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
[GitHub] [arrow] tdcmeehan commented on pull request #37528: GH-37527: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "tdcmeehan (via GitHub)" <gi...@apache.org>.
tdcmeehan commented on PR #37528:
URL: https://github.com/apache/arrow/pull/37528#issuecomment-1703611295
My 0.02 cents: as a user of Arrow, it's not obvious that I should represent a missing schema as a new `Schema` with a 0-length list of fields.
I tried to browse the codebase for conventions I could learn from, and saw `PollInfo` as a source of inspiration: nullable fields are simply passed in null references, and they're exposed externally as Optional absent. Please let me know if I misjudged this project's conventions!
--
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
[GitHub] [arrow] lidavidm commented on pull request #37528: GH-37527: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #37528:
URL: https://github.com/apache/arrow/pull/37528#issuecomment-1703355179
I think there are two reasons:
- Go already allows this, and we should make sure it's exposed and handled consistently across implementations
- No schema is more informative than an empty schema (which I doubt would ever be returned by any database, but is still technically a valid schema)
Protobuf doesn't differentiate a null and an absent schema string, but an empty schema should serialize to a non-null string, so we can still tell the difference, too.
--
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
[GitHub] [arrow] lidavidm commented on pull request #37528: GH-37553: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #37528:
URL: https://github.com/apache/arrow/pull/37528#issuecomment-1711774580
No problem. Feel free to reach out with questions about Flight, since I see there's a discussion going on for Presto
--
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
[GitHub] [arrow] tdcmeehan commented on a diff in pull request #37528: GH-37553: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "tdcmeehan (via GitHub)" <gi...@apache.org>.
tdcmeehan commented on code in PR #37528:
URL: https://github.com/apache/arrow/pull/37528#discussion_r1317831491
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightInfo.java:
##########
@@ -93,7 +93,6 @@ public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoin
*/
public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoint> endpoints, long bytes,
long records, boolean ordered, IpcOption option) {
- Objects.requireNonNull(schema);
Objects.requireNonNull(descriptor);
Objects.requireNonNull(endpoints);
MetadataV4UnionChecker.checkForUnion(schema.getFields().iterator(), option.metadataVersion);
Review Comment:
Thanks for catching, let me look into the unit test.
--
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
[GitHub] [arrow] kou commented on pull request #37528: GH-37527: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #37528:
URL: https://github.com/apache/arrow/pull/37528#issuecomment-1704036757
Thanks. I understand.
Could you create a new sub-issue of GH-37527 (like GH-36620 and GH-36643) only for this (update the Java implementation)?
I think that we also need to update our documents to clarify `FlightInfo::schema` may be `null`. We can defer the task by creating a sub-issue for this.
(Do we need to update the C++ implementation too?)
--
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
[GitHub] [arrow] tdcmeehan commented on pull request #37528: GH-37527: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "tdcmeehan (via GitHub)" <gi...@apache.org>.
tdcmeehan commented on PR #37528:
URL: https://github.com/apache/arrow/pull/37528#issuecomment-1705747116
> Thanks. I understand.
>
> Could you create a new sub-issue of [GH-37527](https://github.com/apache/arrow/issues/37527) (like [GH-36620](https://github.com/apache/arrow/issues/36620) and [GH-36643](https://github.com/apache/arrow/issues/36643)) only for this (update the Java implementation)? I think that we also need to update our documents to clarify `FlightInfo::schema` may be `null`. We can defer the task by creating a sub-issue for this. (Do we need to update the C++ implementation too?)
I created GH-37554 for the docs update, and #37553 specifically for the Java update, and they are part of a task list in GH-37527. I would like some help on the C++ issue. :)
--
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
[GitHub] [arrow] github-actions[bot] commented on pull request #37528: GH-37553: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37528:
URL: https://github.com/apache/arrow/pull/37528#issuecomment-1705748103
:warning: GitHub issue #37553 **has been automatically assigned in GitHub** to PR creator.
--
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
[GitHub] [arrow] tdcmeehan commented on a diff in pull request #37528: GH-37553: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "tdcmeehan (via GitHub)" <gi...@apache.org>.
tdcmeehan commented on code in PR #37528:
URL: https://github.com/apache/arrow/pull/37528#discussion_r1317920918
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightInfo.java:
##########
@@ -93,7 +93,6 @@ public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoin
*/
public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoint> endpoints, long bytes,
long records, boolean ordered, IpcOption option) {
- Objects.requireNonNull(schema);
Objects.requireNonNull(descriptor);
Objects.requireNonNull(endpoints);
MetadataV4UnionChecker.checkForUnion(schema.getFields().iterator(), option.metadataVersion);
Review Comment:
Added a test!
--
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
[GitHub] [arrow] lidavidm commented on a diff in pull request #37528: GH-37553: [Java] Allow FlightInfo#Schema to be nullable for long-running queries
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #37528:
URL: https://github.com/apache/arrow/pull/37528#discussion_r1316039198
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightInfo.java:
##########
@@ -130,8 +132,8 @@ public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoin
option = IpcOption.DEFAULT;
}
- public Schema getSchema() {
- return schema;
+ public Optional<Schema> getSchema() {
Review Comment:
`getSchemaOptional`, perhaps?
--
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