From 8ff0783f93a3c0d0f1861852060197851acef57a Mon Sep 17 00:00:00 2001 From: Devin Smith Date: Wed, 24 Apr 2024 10:48:55 -0700 Subject: [PATCH 1/2] Deprecate implicit String-to-UTF8-bytes ticket methods The implicit String-to-UTF8-bytes ticket methods are potentially confusing when the user really wants to get a query scoped variable ticket. From the perspective of the java client, this may materialize as errors that look something like: ``` Exception in thread "main" io.deephaven.client.impl.TableHandle$UncheckedTableHandleException: io.deephaven.client.impl.TableHandle$TableHandleException: io.grpc.StatusRuntimeException: FAILED_PRECONDITION: Could not resolve 'sourceId': no resolver for route '84' (byte) ``` In this case, we can see that the user likely had some variable name that starts with "t" (ascii 84) and tried to use the variable name directly with one of the String-to-UTF8-bytes ticket methods. This PR aims to deprecates those methods and internally removes any usage that we have on them. --- .../deephaven/engine/table/TableFactory.java | 9 +++++--- .../java/io/deephaven/engine/sql/Sql.java | 8 +++++-- .../deephaven/client/examples/DoPutSpray.java | 4 +++- .../java/io/deephaven/qst/TableCreator.java | 12 ++++++---- .../io/deephaven/qst/table/TableSpec.java | 23 +++++++++++++------ .../io/deephaven/qst/table/TicketTable.java | 6 +++-- .../java/io/deephaven/sql/SqlAdapterTest.java | 17 +++++++++----- 7 files changed, 54 insertions(+), 25 deletions(-) diff --git a/engine/api/src/main/java/io/deephaven/engine/table/TableFactory.java b/engine/api/src/main/java/io/deephaven/engine/table/TableFactory.java index 8532c06552a..f4770c7fa26 100644 --- a/engine/api/src/main/java/io/deephaven/engine/table/TableFactory.java +++ b/engine/api/src/main/java/io/deephaven/engine/table/TableFactory.java @@ -10,6 +10,7 @@ import io.deephaven.qst.table.TicketTable; import io.deephaven.qst.table.TimeTable; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.time.Instant; import java.util.Arrays; @@ -221,14 +222,16 @@ public static Table merge(Table[] tables) { } /** - * Equivalent to {@code of(TicketTable.of(ticket))}. + * Equivalent to {@code of(TicketTable.of(ticket.getBytes(StandardCharsets.UTF_8)))}. * * @param ticket the ticket string * @return the ticket table - * @see TicketTable#of(String) + * @see TicketTable#of(byte[]) + * @deprecated prefer {@link #ticket(byte[])} or */ + @Deprecated public static Table ticket(String ticket) { - return of(TicketTable.of(ticket)); + return of(TicketTable.of(ticket.getBytes(StandardCharsets.UTF_8))); } /** diff --git a/engine/sql/src/main/java/io/deephaven/engine/sql/Sql.java b/engine/sql/src/main/java/io/deephaven/engine/sql/Sql.java index ba3fa7c0863..665a4378ed8 100644 --- a/engine/sql/src/main/java/io/deephaven/engine/sql/Sql.java +++ b/engine/sql/src/main/java/io/deephaven/engine/sql/Sql.java @@ -25,11 +25,11 @@ import io.deephaven.sql.TableInformation; import io.deephaven.util.annotations.ScriptApi; +import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; -import java.util.stream.Collectors; /** * Experimental SQL execution. Subject to change. @@ -64,6 +64,10 @@ private static TableSpec parseSql(String sql, Map scope, Map scope, Map out) { final ScopeStaticImpl.Builder builder = ScopeStaticImpl.builder(); for (Entry e : scope.entrySet()) { @@ -71,7 +75,7 @@ private static Scope scope(Map scope, Map out final Table table = e.getValue(); // The TicketTable can technically be anything unique (incrementing number, random, ...), but for // visualization purposes it makes sense to use the (already unique) table name. - final TicketTable spec = TicketTable.of("sqlref/" + tableName); + final TicketTable spec = sqlref(tableName); final List qualifiedName = List.of(tableName); final TableHeader header = adapt(table.getDefinition()); builder.addTables(TableInformation.of(qualifiedName, header, spec)); diff --git a/java-client/flight-examples/src/main/java/io/deephaven/client/examples/DoPutSpray.java b/java-client/flight-examples/src/main/java/io/deephaven/client/examples/DoPutSpray.java index bb4dd535afa..b06d4171412 100644 --- a/java-client/flight-examples/src/main/java/io/deephaven/client/examples/DoPutSpray.java +++ b/java-client/flight-examples/src/main/java/io/deephaven/client/examples/DoPutSpray.java @@ -16,6 +16,7 @@ import picocli.CommandLine.Command; import picocli.CommandLine.Parameters; +import java.nio.charset.StandardCharsets; import java.util.List; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; @@ -49,7 +50,8 @@ public Void call() throws Exception { .addShutdownHook(new Thread(() -> onShutdown(scheduler, sourceChannel))); final FlightSession sourceSession = session(bufferAllocator, scheduler, sourceChannel); - try (final TableHandle sourceHandle = sourceSession.session().execute(TicketTable.of(ticket))) { + try (final TableHandle sourceHandle = + sourceSession.session().execute(TicketTable.of(ticket.getBytes(StandardCharsets.UTF_8)))) { for (ConnectOptions other : connects.subList(1, connects.size())) { final FlightSession destSession = session(bufferAllocator, scheduler, other.open()); try (final FlightStream in = sourceSession.stream(sourceHandle)) { diff --git a/qst/src/main/java/io/deephaven/qst/TableCreator.java b/qst/src/main/java/io/deephaven/qst/TableCreator.java index eecf72c1baa..30f5a6355f3 100644 --- a/qst/src/main/java/io/deephaven/qst/TableCreator.java +++ b/qst/src/main/java/io/deephaven/qst/TableCreator.java @@ -12,6 +12,7 @@ import io.deephaven.qst.table.TicketTable; import io.deephaven.qst.table.TimeTable; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.time.Instant; import java.util.Arrays; @@ -245,18 +246,21 @@ default TABLE merge(TABLE[] tables) { } /** - * Equivalent to {@code of(TicketTable.of(ticket))}. + * Create a ticket table with the UTF-8 bytes from the {@code ticket} string. Equivalent to + * {@code of(TicketTable.of(ticket.getBytes(StandardCharsets.UTF_8)))}. * * @param ticket the ticket string * @return the ticket table - * @see TicketTable#of(String) + * @see TicketTable#of(byte[]) + * @deprecated prefer {@link #ticket(byte[])} or other explicit methods on {@link TicketTable} */ + @Deprecated default TABLE ticket(String ticket) { - return of(TicketTable.of(ticket)); + return of(TicketTable.of(ticket.getBytes(StandardCharsets.UTF_8))); } /** - * Equivalent to {@code of(TicketTable.of(ticket))}. + * Create a ticket table with the {@code ticket} bytes. Equivalent to {@code of(TicketTable.of(ticket))}. * * @param ticket the ticket * @return the ticket table diff --git a/qst/src/main/java/io/deephaven/qst/table/TableSpec.java b/qst/src/main/java/io/deephaven/qst/table/TableSpec.java index 8a2ff213ad7..f926bb40d39 100644 --- a/qst/src/main/java/io/deephaven/qst/table/TableSpec.java +++ b/qst/src/main/java/io/deephaven/qst/table/TableSpec.java @@ -11,12 +11,7 @@ import io.deephaven.qst.TableCreator.TableToOperations; import org.immutables.value.Value.Derived; -import java.io.BufferedInputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.ObjectInputStream; -import java.nio.file.Files; -import java.nio.file.Path; +import java.nio.charset.StandardCharsets; import java.util.Collection; /** @@ -57,10 +52,24 @@ static TableSpec of(TableCreationLogic logic) { return logic.create(TableCreatorImpl.INSTANCE); } + /** + * Create a ticket table with the UTF-8 bytes from the {@code ticket} string. + * + * @param ticket the ticket + * @return the ticket table + * @deprecated prefer {@link #ticket(byte[])} + */ + @Deprecated static TicketTable ticket(String ticket) { - return TicketTable.of(ticket); + return TicketTable.of(ticket.getBytes(StandardCharsets.UTF_8)); } + /** + * Create a ticket table with the {@code ticket} bytes. + * + * @param ticket the ticket + * @return the ticket table + */ static TicketTable ticket(byte[] ticket) { return TicketTable.of(ticket); } diff --git a/qst/src/main/java/io/deephaven/qst/table/TicketTable.java b/qst/src/main/java/io/deephaven/qst/table/TicketTable.java index 4622b0b2df2..8a63adae944 100644 --- a/qst/src/main/java/io/deephaven/qst/table/TicketTable.java +++ b/qst/src/main/java/io/deephaven/qst/table/TicketTable.java @@ -33,7 +33,9 @@ public static TicketTable of(byte[] ticket) { * * @param ticket the ticket string * @return the ticket table + * @deprecated prefer to be explicit and either use {@link #of(byte[])} or {@link #fromQueryScopeField(String)} */ + @Deprecated public static TicketTable of(String ticket) { return ImmutableTicketTable.of(ticket.getBytes(StandardCharsets.UTF_8)); } @@ -45,7 +47,7 @@ public static TicketTable of(String ticket) { * @return the ticket table */ public static TicketTable fromQueryScopeField(String fieldName) { - return of("s/" + fieldName); + return of(("s/" + fieldName).getBytes(StandardCharsets.UTF_8)); } /** @@ -56,7 +58,7 @@ public static TicketTable fromQueryScopeField(String fieldName) { * @return the ticket table */ public static TicketTable fromApplicationField(String applicationId, String fieldName) { - return of("a/" + applicationId + "/f/" + fieldName); + return of(("a/" + applicationId + "/f/" + fieldName).getBytes(StandardCharsets.UTF_8)); } /** diff --git a/sql/src/test/java/io/deephaven/sql/SqlAdapterTest.java b/sql/src/test/java/io/deephaven/sql/SqlAdapterTest.java index 752bab3a6ea..3e50568ad68 100644 --- a/sql/src/test/java/io/deephaven/sql/SqlAdapterTest.java +++ b/sql/src/test/java/io/deephaven/sql/SqlAdapterTest.java @@ -12,6 +12,7 @@ import java.io.IOException; import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.List; @@ -302,9 +303,13 @@ private static Scope scope() { return ScopeStaticImpl.empty(); } + private static TicketTable scan(String name) { + return TicketTable.of(("scan/" + name).getBytes(StandardCharsets.UTF_8)); + } + private static Scope scope(String name, TableHeader header) { return ScopeStaticImpl.builder() - .addTables(TableInformation.of(List.of(name), header, TicketTable.of("scan/" + name))) + .addTables(TableInformation.of(List.of(name), header, scan(name))) .build(); } @@ -313,8 +318,8 @@ private static Scope scope( String name2, TableHeader header2) { return ScopeStaticImpl.builder() .addTables( - TableInformation.of(List.of(name1), header1, TicketTable.of("scan/" + name1)), - TableInformation.of(List.of(name2), header2, TicketTable.of("scan/" + name2))) + TableInformation.of(List.of(name1), header1, scan(name1)), + TableInformation.of(List.of(name2), header2, scan(name2))) .build(); } @@ -324,9 +329,9 @@ private static Scope scope( String name3, TableHeader header3) { return ScopeStaticImpl.builder() .addTables( - TableInformation.of(List.of(name1), header1, TicketTable.of("scan/" + name1)), - TableInformation.of(List.of(name2), header2, TicketTable.of("scan/" + name2)), - TableInformation.of(List.of(name3), header3, TicketTable.of("scan/" + name3))) + TableInformation.of(List.of(name1), header1, scan(name1)), + TableInformation.of(List.of(name2), header2, scan(name2)), + TableInformation.of(List.of(name3), header3, scan(name3))) .build(); } } From 0c8c14eecc44ce7d69e081c5139e8c89d168d4fd Mon Sep 17 00:00:00 2001 From: Devin Smith Date: Wed, 24 Apr 2024 10:59:07 -0700 Subject: [PATCH 2/2] cleanup doc --- .../src/main/java/io/deephaven/engine/table/TableFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/api/src/main/java/io/deephaven/engine/table/TableFactory.java b/engine/api/src/main/java/io/deephaven/engine/table/TableFactory.java index f4770c7fa26..eebff83fa69 100644 --- a/engine/api/src/main/java/io/deephaven/engine/table/TableFactory.java +++ b/engine/api/src/main/java/io/deephaven/engine/table/TableFactory.java @@ -227,7 +227,7 @@ public static Table merge(Table[] tables) { * @param ticket the ticket string * @return the ticket table * @see TicketTable#of(byte[]) - * @deprecated prefer {@link #ticket(byte[])} or + * @deprecated prefer {@link #ticket(byte[])} */ @Deprecated public static Table ticket(String ticket) {