Skip to content

Commit

Permalink
Revert "Add query timeout to database connection (#3218)" (#3247)
Browse files Browse the repository at this point in the history 
This reverts commit 01e4528.
  • Loading branch information
@gs-ssh16
gs-ssh16 authored Nov 13, 2024
1 parent ef4cef5 commit 874ecec
Show file tree
Hide file tree
Showing 24 changed files with 16 additions and 222 deletions.
2 changes: 1 addition & 1 deletion ...ain/java/org/finos/legend/engine/plan/execution/stores/relational/RelationalExecutor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ public SQLUpdateResult execute(RelationalSaveNode node, Identity identity, Execu

this.prepareForSQLExecution(node.sqlQuery, node.sqlComment, connectionManagerConnection, databaseTimeZone, databaseType, tempTableList, identity, executionState, false);

return new SQLUpdateResult(executionState.activities, databaseType, connectionManagerConnection, node.connection, identity, tempTableList, executionState.getRequestContext());
return new SQLUpdateResult(executionState.activities, databaseType, connectionManagerConnection, identity, tempTableList, executionState.getRequestContext());
}

private void prepareForSQLExecution(String sqlQuery, String sqlComment, Connection connection, String databaseTimeZone, String databaseTypeName, List<String> tempTableList, Identity identity, ExecutionState executionState, boolean shouldLogSQL)
Expand Down
1 change: 0 additions & 1 deletion ...engine/plan/execution/stores/relational/connection/manager/ConnectionManagerSelector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ public static org.finos.legend.engine.protocol.pure.v1.model.packageableElement.
db.type = DatabaseType.H2;
db.element = originalConnection.element;
db.timeZone = originalConnection instanceof DatabaseConnection ? ((DatabaseConnection) originalConnection).timeZone : null;
db.queryTimeOutInSeconds = originalConnection instanceof DatabaseConnection ? ((DatabaseConnection) originalConnection).queryTimeOutInSeconds : null;
db.quoteIdentifiers = originalConnection instanceof DatabaseConnection ? ((DatabaseConnection) originalConnection).quoteIdentifiers : null;
return db;
}
Expand Down
2 changes: 1 addition & 1 deletion ...a/org/finos/legend/engine/plan/execution/stores/relational/result/SQLExecutionResult.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public SQLExecutionResult(List<ExecutionActivity> activities, SQLExecutionNode S

public SQLExecutionResult(List<ExecutionActivity> activities, SQLExecutionNode SQLExecutionNode, String databaseType, String databaseTimeZone, Connection connection, Identity identity, List<String> temporaryTables, Span topSpan, RequestContext requestContext, boolean logSQLWithParamValues)
{
super("success", connection, SQLExecutionNode.connection, activities, databaseType, temporaryTables, requestContext);
super("success", connection, activities, databaseType, temporaryTables, requestContext);
this.SQLExecutionNode = SQLExecutionNode;
this.databaseTimeZone = databaseTimeZone;
this.calendar = new GregorianCalendar(TimeZone.getTimeZone(databaseTimeZone));
Expand Down
7 changes: 1 addition & 6 deletions .../main/java/org/finos/legend/engine/plan/execution/stores/relational/result/SQLResult.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.finos.legend.engine.plan.execution.stores.StoreExecutable;
import org.finos.legend.engine.plan.execution.stores.relational.connection.driver.DatabaseManager;
import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.connection.DatabaseType;
import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.connection.DatabaseConnection;
import org.finos.legend.engine.shared.core.api.request.RequestContext;
import org.finos.legend.engine.shared.core.identity.Identity;
import org.finos.legend.engine.shared.core.identity.factory.*;
Expand All @@ -46,7 +45,7 @@ public abstract class SQLResult extends Result implements StoreExecutable

private final RequestContext requestContext;

public SQLResult(String status, Connection connection, DatabaseConnection protocolConnection, List<ExecutionActivity> activities, String databaseType, List<String> temporaryTables, RequestContext requestContext)
public SQLResult(String status, Connection connection, List<ExecutionActivity> activities, String databaseType, List<String> temporaryTables, RequestContext requestContext)
{
super(status, activities);

Expand All @@ -62,10 +61,6 @@ public SQLResult(String status, Connection connection, DatabaseConnection protoc
{
this.statement.setFetchSize(100);
}
if (protocolConnection.queryTimeOutInSeconds != null)
{
this.statement.setQueryTimeout(protocolConnection.queryTimeOutInSeconds);
}
}
catch (Throwable e)
{
Expand Down
5 changes: 2 additions & 3 deletions ...java/org/finos/legend/engine/plan/execution/stores/relational/result/SQLUpdateResult.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import org.finos.legend.engine.plan.execution.stores.relational.activity.RelationalExecutionActivity;
import org.finos.legend.engine.shared.core.api.request.RequestContext;
import org.finos.legend.engine.shared.core.identity.Identity;
import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.connection.DatabaseConnection;
import org.finos.legend.engine.shared.core.operational.logs.LogInfo;
import org.finos.legend.engine.shared.core.operational.logs.LoggingEventType;
import org.slf4j.Logger;
Expand All @@ -33,9 +32,9 @@ public class SQLUpdateResult extends SQLResult

private final int updateCount;

public SQLUpdateResult(List<ExecutionActivity> activities, String databaseType, Connection connection, DatabaseConnection dbConnection, Identity identity, List<String> temporaryTables, RequestContext requestContext)
public SQLUpdateResult(List<ExecutionActivity> activities, String databaseType, Connection connection, Identity identity, List<String> temporaryTables, RequestContext requestContext)
{
super("success", connection, dbConnection, activities, databaseType, temporaryTables, requestContext);
super("success", connection, activities, databaseType, temporaryTables, requestContext);
try
{
long start = System.currentTimeMillis();
Expand Down
137 changes: 0 additions & 137 deletions ...org/finos/legend/engine/plan/execution/stores/relational/connection/TestQueryTimeOut.java
View file

This file was deleted.

1 change: 0 additions & 1 deletion .../language/pure/grammar/from/antlr4/connection/RelationalDatabaseConnectionLexerGrammar.g4
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ MODE: 'mode';
RELATIONAL_DATASOURCE_SPEC: 'specification';
RELATIONAL_AUTH_STRATEGY: 'auth';
RELATIONAL_POST_PROCESSORS: 'postProcessors';
QUERY_TIMEOUT: 'queryTimeOutInSeconds';

DB_TIMEZONE: 'timezone';
TIMEZONE: TimeZone;
Expand Down
8 changes: 2 additions & 6 deletions ...language/pure/grammar/from/antlr4/connection/RelationalDatabaseConnectionParserGrammar.g4
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ options

identifier: VALID_STRING | STRING
| STORE
| TYPE | QUERY_TIMEOUT | RELATIONAL_DATASOURCE_SPEC | RELATIONAL_AUTH_STRATEGY
| TYPE | RELATIONAL_DATASOURCE_SPEC | RELATIONAL_AUTH_STRATEGY
| DB_TIMEZONE | QUOTE_IDENTIFIERS
;

Expand All @@ -22,7 +22,6 @@ identifier: VALID_STRING | STRING
definition: (
connectionStore
| dbType
| queryTimeOutInSeconds
| connectionMode
| dbConnectionTimezone
| dbQuoteIdentifiers
Expand All @@ -40,8 +39,6 @@ dbQuoteIdentifiers: QUOTE_IDENTIFIERS COLON BOOLEAN SEMI_COL
;
dbType: TYPE COLON identifier SEMI_COLON
;
queryTimeOutInSeconds: QUERY_TIMEOUT COLON INTEGER SEMI_COLON
;

connectionMode: MODE COLON identifier SEMI_COLON
;
Expand Down Expand Up @@ -70,5 +67,4 @@ specificationValueBody: BRACE_OPEN (specificationValue)*
;

specificationValue: SPECIFICATION_BRACE_OPEN | SPECIFICATION_CONTENT | SPECIFICATION_BRACE_CLOSE
;

;
6 changes: 1 addition & 5 deletions .../engine/language/pure/compiler/toPureGraph/HelperRelationalDatabaseConnectionBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,10 @@ public static void addTestDataSetUp(Root_meta_external_store_relational_runtime_
}
}

public static void addDatabaseConnectionProperties(Root_meta_external_store_relational_runtime_DatabaseConnection pureConnection, String element, SourceInformation elementSourceInformation, String connectionType, String timeZone, Integer queryTimeOutInSeconds, Boolean quoteIdentifiers, CompileContext context)
public static void addDatabaseConnectionProperties(Root_meta_external_store_relational_runtime_DatabaseConnection pureConnection, String element, SourceInformation elementSourceInformation, String connectionType, String timeZone, Boolean quoteIdentifiers, CompileContext context)
{
Root_meta_external_store_relational_runtime_DatabaseConnection connection = pureConnection._type(context.pureModel.getEnumValue("meta::relational::runtime::DatabaseType", connectionType));
connection._timeZone(timeZone);
if (queryTimeOutInSeconds != null)
{
connection._queryTimeOutInSeconds(Long.valueOf(queryTimeOutInSeconds));
}
connection._quoteIdentifiers(quoteIdentifiers);
if (element != null)
{
Expand Down
2 changes: 1 addition & 1 deletion ...g/finos/legend/engine/language/pure/compiler/toPureGraph/RelationalCompilerExtension.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ public List<Function2<Connection, CompileContext, Root_meta_core_runtime_Connect
RelationalDatabaseConnection relationalDatabaseConnection = (RelationalDatabaseConnection) connectionValue;

Root_meta_external_store_relational_runtime_RelationalDatabaseConnection relational = new Root_meta_external_store_relational_runtime_RelationalDatabaseConnection_Impl("", SourceInformationHelper.toM3SourceInformation(relationalDatabaseConnection.sourceInformation), context.pureModel.getClass("meta::external::store::relational::runtime::RelationalDatabaseConnection"));
HelperRelationalDatabaseConnectionBuilder.addDatabaseConnectionProperties(relational, relationalDatabaseConnection.element, relationalDatabaseConnection.elementSourceInformation, relationalDatabaseConnection.type.name(), relationalDatabaseConnection.timeZone, relationalDatabaseConnection.queryTimeOutInSeconds, relationalDatabaseConnection.quoteIdentifiers, context);
HelperRelationalDatabaseConnectionBuilder.addDatabaseConnectionProperties(relational, relationalDatabaseConnection.element, relationalDatabaseConnection.elementSourceInformation, relationalDatabaseConnection.type.name(), relationalDatabaseConnection.timeZone, relationalDatabaseConnection.quoteIdentifiers, context);

List<IRelationalCompilerExtension> extensions = IRelationalCompilerExtension.getExtensions(context);

Expand Down
3 changes: 0 additions & 3 deletions ...legend/engine/language/pure/grammar/from/RelationalDatabaseConnectionParseTreeWalker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,6 @@ public void visitRelationalDatabaseConnectionValue(RelationalDatabaseConnectionP
{
throw new EngineException("Unknown database type '" + PureGrammarParserUtility.fromIdentifier(dbTypeCtx.identifier()) + "'", this.walkerSourceInformation.getSourceInformation(dbTypeCtx), EngineErrorType.PARSER);
}
//queryTimeoutInSeconds (optional)
RelationalDatabaseConnectionParserGrammar.QueryTimeOutInSecondsContext queryTimeOutInSecondsCtx = PureGrammarParserUtility.validateAndExtractOptionalField(ctx.queryTimeOutInSeconds(), "queryTimeOutInSeconds", connectionValue.sourceInformation);
connectionValue.queryTimeOutInSeconds = queryTimeOutInSecondsCtx != null ? Integer.parseInt(queryTimeOutInSecondsCtx.INTEGER().getText()) : null;
// timezone (optional)
RelationalDatabaseConnectionParserGrammar.DbConnectionTimezoneContext timezoneCtx = PureGrammarParserUtility.validateAndExtractOptionalField(ctx.dbConnectionTimezone(), "timezone", connectionValue.sourceInformation);
connectionValue.timeZone = timezoneCtx != null ? timezoneCtx.TIMEZONE().getText() : null;
Expand Down
1 change: 0 additions & 1 deletion .../org/finos/legend/engine/language/pure/grammar/to/RelationalGrammarComposerExtension.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,6 @@ public List<Function2<Connection, PureGrammarComposerContext, Pair<String, Strin
context.getIndentationString() + getTabString(baseIndentation + 1) + "specification: " + specification + ";\n" +
context.getIndentationString() + getTabString(baseIndentation + 1) + "auth: " + authenticationStrategy + ";\n"
)) +
(relationalDatabaseConnection.queryTimeOutInSeconds != null ? (context.getIndentationString() + getTabString(baseIndentation + 1) + "queryTimeOutInSeconds: " + String.valueOf(relationalDatabaseConnection.queryTimeOutInSeconds) + ";\n") : "") +
(postProcessors != null
? context.getIndentationString() + getTabString(baseIndentation + 1) + postProcessors
: "") +
Expand Down
4 changes: 0 additions & 4 deletions ...gend/engine/language/pure/compiler/test/TestRelationalConnectionCompilationRoundtrip.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ public void testSqlServerConnectionPropertiesPropagatedToCompiledGraph()
" userNameVaultReference: 'value';\n" +
" passwordVaultReference: 'value';\n" +
" };\n" +
" queryTimeOutInSeconds: 7777;\n" +
"}\n");
}

Expand Down Expand Up @@ -139,13 +138,10 @@ public void testH2ConnectionPropertiesPropagatedToCompiledGraph()
" auth: Test\n" +
" {\n" +
" };\n" +
" queryTimeOutInSeconds: 7777;\n" +
"}\n");
Root_meta_external_store_relational_runtime_RelationalDatabaseConnection connection = (Root_meta_external_store_relational_runtime_RelationalDatabaseConnection) compiledGraph.getTwo().getConnection("simple::H2Connection", SourceInformation.getUnknownSourceInformation());
Boolean quoteIdentifiers = connection._quoteIdentifiers();
Long expectedQueryTimeOutInSeconds = 7777L;

Assert.assertTrue(quoteIdentifiers);
Assert.assertEquals(expectedQueryTimeOutInSeconds, connection._queryTimeOutInSeconds());
}
}
14 changes: 0 additions & 14 deletions ...finos/legend/engine/language/pure/grammar/test/TestRelationalConnectionGrammarParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,20 +66,6 @@ private String getTemplateConnectionWithTz(String offsetOrCode)
"}\n\n";
}

@Test
public void testConnectionWithTimeOut()
{
test("###Connection\n" +
"RelationalDatabaseConnection meta::mySimpleConnection\n" +
"{\n" +
" store: model::firm::Person;\n" +
" queryTimeOutInSeconds: 5000;\n" +
" type: H2;\n" +
" specification: LocalH2 { testDataSetupCSV: 'testCSV'; };\n" +
" auth: DefaultH2;\n" +
"}\n\n");
}

@Test
public void testTimezoneConfiguration()
{
Expand Down
Loading

0 comments on commit 874ecec

Please sign in to comment.