Skip to content

Commit 1369797

Browse files
Karthik KrishnaswamyKarthik Krishnaswamy
authored andcommitted
Add semantic warning logic for non-deterministic CTE reuse
:wq :wq
1 parent 8217319 commit 1369797

File tree

19 files changed

+388
-325
lines changed

19 files changed

+388
-325
lines changed

presto-cli/src/main/java/com/facebook/presto/cli/Console.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import com.facebook.airlift.log.LoggingConfiguration;
1818
import com.facebook.presto.cli.ClientOptions.OutputFormat;
1919
import com.facebook.presto.client.ClientSession;
20+
import com.facebook.presto.spi.PrestoWarning;
2021
import com.facebook.presto.spi.security.SelectedRole;
2122
import com.facebook.presto.sql.parser.StatementSplitter;
2223
import com.google.common.annotations.VisibleForTesting;
@@ -37,6 +38,7 @@
3738
import java.io.IOException;
3839
import java.io.PrintStream;
3940
import java.util.HashMap;
41+
import java.util.List;
4042
import java.util.Map;
4143
import java.util.Optional;
4244
import java.util.concurrent.CountDownLatch;
@@ -316,6 +318,19 @@ private static boolean process(QueryRunner queryRunner, String sql, OutputFormat
316318

317319
try (Query query = queryRunner.startQuery(finalSql)) {
318320
boolean success = query.renderOutput(System.out, outputFormat, interactive);
321+
try {
322+
List<PrestoWarning> warnings = query.getWarnings();
323+
if (warnings != null && !warnings.isEmpty()) {
324+
for (PrestoWarning warning : warnings) {
325+
System.err.println("WARNING: " + warning.getMessage());
326+
}
327+
}
328+
}
329+
catch (Exception e) {
330+
System.err.println("Failed to retrieve warnings: " + e.getMessage());
331+
}
332+
333+
}
319334

320335
ClientSession session = queryRunner.getSession();
321336

presto-cli/src/main/java/com/facebook/presto/cli/Query.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,15 @@ public class Query
5858
private final AtomicBoolean ignoreUserInterrupt = new AtomicBoolean();
5959
private final StatementClient client;
6060
private final boolean debug;
61+
private final List<PrestoWarning> warnings;
62+
public Query(StatementClient client, boolean debug, boolean runtime) {
63+
this.client = requireNonNull(client, "client is null");
64+
this.debug = debug;
65+
this.runtime = runtime;
66+
this.warnings = client.getWarnings();
67+
}
68+
69+
6170
private final boolean runtime;
6271
private Optional<Long> clientStopTimestamp = Optional.empty();
6372

@@ -409,6 +418,9 @@ private static void renderErrorLocation(String query, ErrorLocation location, Pr
409418
}
410419
}
411420

421+
public PrestoWarning[] getWarnings() {
422+
}
423+
412424
private static class PrintStreamWarningsPrinter
413425
extends AbstractWarningsPrinter
414426
{

presto-client/src/main/java/com/facebook/presto/client/QueryResults.java

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,7 @@
3333

3434
@Immutable
3535
public class QueryResults
36-
implements QueryStatusInfo, QueryData
37-
{
36+
implements QueryStatusInfo, QueryData {
3837
private final String id;
3938
private final URI infoUri;
4039
private final URI partialCancelUri;
@@ -48,6 +47,7 @@ public class QueryResults
4847
private final String updateType;
4948
private final Long updateCount;
5049

50+
5151
@JsonCreator
5252
public QueryResults(
5353
@JsonProperty("id") String id,
@@ -61,8 +61,7 @@ public QueryResults(
6161
@JsonProperty("error") QueryError error,
6262
@JsonProperty("warnings") List<PrestoWarning> warnings,
6363
@JsonProperty("updateType") String updateType,
64-
@JsonProperty("updateCount") Long updateCount)
65-
{
64+
@JsonProperty("updateCount") Long updateCount) {
6665
this(
6766
id,
6867
infoUri,
@@ -90,8 +89,7 @@ public QueryResults(
9089
QueryError error,
9190
List<PrestoWarning> warnings,
9291
String updateType,
93-
Long updateCount)
94-
{
92+
Long updateCount) {
9593
this.id = requireNonNull(id, "id is null");
9694
this.infoUri = requireNonNull(infoUri, "infoUri is null");
9795
this.partialCancelUri = partialCancelUri;
@@ -219,6 +217,7 @@ public List<PrestoWarning> getWarnings()
219217
return warnings;
220218
}
221219

220+
222221
/**
223222
* Returns the update type, if any, of the query as determined by the Analyzer
224223
* Could be INSERT, DELETE, CREATE, etc.

presto-client/src/main/java/com/facebook/presto/client/StatementClient.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,13 @@
1414
package com.facebook.presto.client;
1515

1616
import com.facebook.presto.common.type.TimeZoneKey;
17+
import com.facebook.presto.spi.PrestoWarning;
1718
import com.facebook.presto.spi.security.SelectedRole;
1819

1920
import javax.annotation.Nullable;
2021

2122
import java.io.Closeable;
23+
import java.util.List;
2224
import java.util.Map;
2325
import java.util.Optional;
2426
import java.util.Set;
@@ -39,13 +41,16 @@ public interface StatementClient
3941
boolean isFinished();
4042

4143
StatementStats getStats();
44+
List<PrestoWarning> getWarnings();
45+
4246

4347
QueryStatusInfo currentStatusInfo();
4448

4549
QueryData currentData();
4650

4751
QueryStatusInfo finalStatusInfo();
4852

53+
4954
Optional<String> getSetCatalog();
5055

5156
Optional<String> getSetSchema();

presto-client/src/main/java/com/facebook/presto/client/StatementClientV1.java

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,12 @@
1616
import com.facebook.airlift.json.JsonCodec;
1717
import com.facebook.presto.client.OkHttpUtil.NullCallback;
1818
import com.facebook.presto.common.type.TimeZoneKey;
19+
import com.facebook.presto.spi.PrestoWarning;
1920
import com.facebook.presto.spi.security.SelectedRole;
2021
import com.google.common.base.Joiner;
2122
import com.google.common.base.Splitter;
23+
import com.facebook.presto.client.QueryResults;
24+
import com.google.common.collect.ImmutableList;
2225
import com.google.common.collect.ImmutableMap;
2326
import com.google.common.collect.ImmutableSet;
2427
import io.airlift.units.Duration;
@@ -98,6 +101,7 @@ class StatementClientV1
98101

99102
private final OkHttpClient httpClient;
100103
private final String query;
104+
private List<PrestoWarning> warnings = ImmutableList.of();
101105
private final AtomicReference<QueryResults> currentResults = new AtomicReference<>();
102106
private final AtomicReference<String> setCatalog = new AtomicReference<>();
103107
private final AtomicReference<String> setSchema = new AtomicReference<>();
@@ -134,7 +138,9 @@ public StatementClientV1(OkHttpClient httpClient, ClientSession session, String
134138

135139
Request request = buildQueryRequest(session, query);
136140

137-
JsonResponse<QueryResults> response = JsonResponse.execute(QUERY_RESULTS_CODEC, httpClient, request);
141+
JsonResponse<com.facebook.presto.client.QueryResults> response = JsonResponse.execute(QUERY_RESULTS_CODEC, httpClient, request);
142+
System.err.println("RAW RESPONSE BODY:");
143+
System.err.println(response.getResponseBody());
138144
if ((response.getStatusCode() != HTTP_OK) || !response.hasValue()) {
139145
state.compareAndSet(State.RUNNING, State.CLIENT_ERROR);
140146
throw requestFailedException("starting query", request, response);
@@ -492,6 +498,10 @@ private void processResponse(Headers headers, QueryResults results)
492498
}
493499

494500
currentResults.set(results);
501+
results = currentResults.get();
502+
if (results != null && results.getWarnings() != null) {
503+
this.warnings = results.getWarnings();
504+
}
495505
}
496506

497507
private RuntimeException requestFailedException(String task, Request request, JsonResponse<QueryResults> response)
@@ -526,6 +536,11 @@ public void cancelLeafStage()
526536
httpDelete(uri);
527537
}
528538
}
539+
@Override
540+
public List<PrestoWarning> getWarnings()
541+
{
542+
return warnings;
543+
}
529544

530545
@Override
531546
public void close()

presto-hive/src/test/java/com/facebook/presto/hive/HiveQueryRunner.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,7 @@
4646
import java.net.URI;
4747
import java.nio.file.Path;
4848
import java.nio.file.Paths;
49-
import java.util.Collections;
50-
import java.util.HashMap;
51-
import java.util.List;
52-
import java.util.Map;
53-
import java.util.Optional;
49+
import java.util.*;
5450
import java.util.function.BiFunction;
5551

5652
import static com.facebook.airlift.log.Level.ERROR;
@@ -464,6 +460,8 @@ public static Session createMaterializeExchangesSession(Optional<SelectedRole> r
464460
public static void main(String[] args)
465461
throws Exception
466462
{
463+
TimeZone.setDefault(TimeZone.getTimeZone("America/Bahia_Banderas"));
464+
System.out.println("JVM timezone = " + java.util.TimeZone.getDefault().getID());
467465
// You need to add "--user user" to your CLI for your queries to work
468466
setupLogging();
469467
Optional<Path> dataDirectory;

0 commit comments

Comments
 (0)