-
Notifications
You must be signed in to change notification settings - Fork 188
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Coral-Service] Add rewriteType field to translation endpoint #455
Changes from 2 commits
8127236
10c49e0
df7a0e5
1a36e53
61c3427
f81e20d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,7 +25,9 @@ | |
import com.linkedin.coral.coralservice.entity.IncrementalRequestBody; | ||
import com.linkedin.coral.coralservice.entity.IncrementalResponseBody; | ||
import com.linkedin.coral.coralservice.entity.TranslateRequestBody; | ||
import com.linkedin.coral.coralservice.utils.RewriteType; | ||
|
||
import static com.linkedin.coral.coralservice.utils.CommonUtils.*; | ||
import static com.linkedin.coral.coralservice.utils.CoralProvider.*; | ||
import static com.linkedin.coral.coralservice.utils.IncrementalUtils.*; | ||
import static com.linkedin.coral.coralservice.utils.TranslationUtils.*; | ||
|
@@ -59,32 +61,35 @@ public ResponseEntity translate(@RequestBody TranslateRequestBody translateReque | |
final String sourceLanguage = translateRequestBody.getSourceLanguage(); | ||
final String targetLanguage = translateRequestBody.getTargetLanguage(); | ||
final String query = translateRequestBody.getQuery(); | ||
final RewriteType rewriteType = translateRequestBody.getRewriteType(); | ||
|
||
// TODO: Allow translations between the same language | ||
if (sourceLanguage.equalsIgnoreCase(targetLanguage)) { | ||
return ResponseEntity.status(HttpStatus.BAD_REQUEST) | ||
.body("Please choose different languages to translate between.\n"); | ||
} | ||
|
||
if (!isValidSourceLanguage(sourceLanguage)) { | ||
return ResponseEntity.status(HttpStatus.BAD_REQUEST) | ||
.body("Currently, only Hive, and Trino are supported as source/input languages.\n"); | ||
} | ||
|
||
String translatedSql = null; | ||
|
||
try { | ||
// TODO: add more translations once n-to-one-to-n is completed | ||
// From Trino | ||
if (sourceLanguage.equalsIgnoreCase("trino")) { | ||
// To Spark | ||
if (targetLanguage.equalsIgnoreCase("spark")) { | ||
translatedSql = translateTrinoToSpark(query); | ||
} | ||
} | ||
// From Hive | ||
else if (sourceLanguage.equalsIgnoreCase("hive")) { | ||
// To Spark | ||
if (targetLanguage.equalsIgnoreCase("spark")) { | ||
translatedSql = translateHiveToSpark(query); | ||
} | ||
// To Trino | ||
else if (targetLanguage.equalsIgnoreCase("trino")) { | ||
translatedSql = translateHiveToTrino(query); | ||
if (rewriteType == null) { | ||
// Invalid rewriteType values are deserialized as null | ||
translatedSql = translateQuery(query, sourceLanguage, targetLanguage); | ||
} else { | ||
switch (rewriteType) { | ||
case INCREMENTAL: | ||
translatedSql = getIncrementalQuery(query, sourceLanguage, targetLanguage); | ||
break; | ||
case DATAMASKING: | ||
case NONE: | ||
default: | ||
translatedSql = translateQuery(query, sourceLanguage, targetLanguage); | ||
break; | ||
} | ||
} | ||
} catch (Throwable t) { | ||
|
@@ -117,6 +122,7 @@ public ResponseEntity getIncrementalInfo(@RequestBody IncrementalRequestBody inc | |
incrementalResponseBody.setIncrementalQuery(null); | ||
try { | ||
if (language.equalsIgnoreCase("spark")) { | ||
// TODO: replace getSparkIncrementalQueryFromUserSql with getIncrementalQuery | ||
String incrementalQuery = getSparkIncrementalQueryFromUserSql(query); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does this work when the source/target specified by the user is not Spark? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still not quite clear. Why are not we using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In order to use Since changes to this endpoint could break the existing DBT integration, it might be worth it to make this it's own separate effort especially since this is a more complicated endpoint to test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we hardcode "spark" and get rid of |
||
for (String tableName : tableNames) { | ||
/* Generate underscore delimited and incremental table names | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,7 @@ | |
import com.linkedin.coral.coralservice.utils.RewriteType; | ||
import com.linkedin.coral.coralservice.utils.VisualizationUtils; | ||
|
||
import static com.linkedin.coral.coralservice.utils.CommonUtils.*; | ||
import static com.linkedin.coral.coralservice.utils.VisualizationUtils.*; | ||
|
||
|
||
|
@@ -43,9 +44,9 @@ public ResponseEntity getIRVisualizations(@RequestBody VisualizationRequestBody | |
final String query = visualizationRequestBody.getQuery(); | ||
final RewriteType rewriteType = visualizationRequestBody.getRewriteType(); | ||
|
||
if (!visualizationUtils.isValidSourceLanguage(sourceLanguage)) { | ||
if (!isValidSourceLanguage(sourceLanguage)) { | ||
return ResponseEntity.status(HttpStatus.BAD_REQUEST) | ||
.body("Currently, only Hive, Spark, and Trino are supported as engines to generate graphs using.\n"); | ||
.body("Currently, only Hive, and Trino are supported as engines to generate graphs using.\n"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let us allow Spark and route it to Hive. Also you can say There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we also do this for the translation endpoint? |
||
} | ||
|
||
// A list of UUIDs in this order of: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
/** | ||
* Copyright 2023 LinkedIn Corporation. All rights reserved. | ||
* Licensed under the BSD-2 Clause license. | ||
* See LICENSE in the project root for license information. | ||
*/ | ||
package com.linkedin.coral.coralservice.utils; | ||
|
||
public class CommonUtils { | ||
|
||
public static boolean isValidSourceLanguage(String sourceLanguage) { | ||
return sourceLanguage.equalsIgnoreCase("trino") || sourceLanguage.equalsIgnoreCase("hive"); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for
/input
.