Skip to content

Commit

Permalink
feat: Add includeLabelInSql configuration option to include query lab…
Browse files Browse the repository at this point in the history
…el as inline comment in generated sql select (#3362)

* feat: Add includeLabelInSql configuration option to include query label as inline comment in generated sql select

The generated select queries can look like:

select /* MyInnerTest.insert_and_find */ t0.id, ...

Using either query.setLabel() or profile location (which all query beans get by default). This means that tooling looking at sql in the database can more easily relate that sql back to application code.

* More tests for labels with includeLabelInSql=true

* Remove trim when using profileLocation label
  • Loading branch information
rbygrave authored Mar 17, 2024
1 parent 02dafc9 commit 8b858a0
Show file tree
Hide file tree
Showing 16 changed files with 129 additions and 56 deletions.
15 changes: 15 additions & 0 deletions ebean-api/src/main/java/io/ebean/DatabaseBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -2209,6 +2209,15 @@ default DatabaseBuilder loadModuleInfo(boolean loadModuleInfo) {
@Deprecated
DatabaseBuilder setLoadModuleInfo(boolean loadModuleInfo);

/**
* Set if generated SQL SELECT should include the query label as an
* inline SQL comment (to help reference back from the SQL to the code
* that executed the query.
*
* @param includeLabelInSql When true include a SQL inline comment in generated SELECT queries.
*/
DatabaseConfig includeLabelInSql(boolean includeLabelInSql);

/**
* Set the naming convention to apply to metrics names.
*/
Expand Down Expand Up @@ -3104,6 +3113,12 @@ interface Settings extends DatabaseBuilder {
*/
boolean isLoadModuleInfo();

/**
* Return true if generated sql select query should include an inline sql comment with the
* query label or profile location label.
*/
boolean isIncludeLabelInSql();

/**
* Return the naming convention to apply to metrics names.
*/
Expand Down
18 changes: 18 additions & 0 deletions ebean-api/src/main/java/io/ebean/config/DatabaseConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,12 @@ public class DatabaseConfig implements DatabaseBuilder.Settings {
*/
private boolean loadModuleInfo = true;

/**
* When true then include a sql comment in generated SELECT queries with the query
* label or profile location label.
*/
private boolean includeLabelInSql;

/**
* Interesting classes such as entities, embedded, ScalarTypes,
* Listeners, Finders, Controllers, AttributeConverters etc.
Expand Down Expand Up @@ -2133,6 +2139,7 @@ protected void loadSettings(PropertiesWrapper p) {
readOnlyDatabase = p.getBoolean("readOnlyDatabase", readOnlyDatabase);
autoPersistUpdates = p.getBoolean("autoPersistUpdates", autoPersistUpdates);
loadModuleInfo = p.getBoolean("loadModuleInfo", loadModuleInfo);
includeLabelInSql = p.getBoolean("includeLabelInSql", includeLabelInSql);
maxCallStack = p.getInt("maxCallStack", maxCallStack);
dumpMetricsOnShutdown = p.getBoolean("dumpMetricsOnShutdown", dumpMetricsOnShutdown);
dumpMetricsOptions = p.get("dumpMetricsOptions", dumpMetricsOptions);
Expand Down Expand Up @@ -2547,6 +2554,11 @@ public boolean isLoadModuleInfo() {
return loadModuleInfo;
}

@Override
public boolean isIncludeLabelInSql() {
return includeLabelInSql;
}

/**
* @deprecated - migrate to {@link #isLoadModuleInfo()}.
*/
Expand All @@ -2563,6 +2575,12 @@ public DatabaseConfig setLoadModuleInfo(boolean loadModuleInfo) {
return this;
}

@Override
public DatabaseConfig includeLabelInSql(boolean includeLabelInSql) {
this.includeLabelInSql = includeLabelInSql;
return this;
}

@Override
public Function<String, String> getMetricNaming() {
return metricNaming;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -769,4 +769,11 @@ protected void escapeLikeCharacter(char ch, StringBuilder sb) {
public boolean supportsNativeJavaTime() {
return supportsNativeJavaTime;
}

public String inlineSqlComment(String label) {
if (label == null) {
return "";
}
return "/* " + label + " */ ";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ void testLoadWithProperties() {
props.setProperty("skipDataSourceCheck", "true");
props.setProperty("readOnlyDatabase", "true");
props.setProperty("lengthCheck", "ON");
props.setProperty("includeLabelInSql", "true");

props.setProperty("queryPlan.enable", "true");
props.setProperty("queryPlan.thresholdMicros", "10000");
Expand All @@ -96,6 +97,7 @@ void testLoadWithProperties() {
assertTrue(settings.isLoadModuleInfo());
assertTrue(settings.skipDataSourceCheck());
assertTrue(settings.readOnlyDatabase());
assertTrue(settings.isIncludeLabelInSql());
assertThat(settings.getLengthCheck()).isEqualTo(LengthCheck.ON);

assertTrue(settings.isIdGeneratorAutomatic());
Expand Down Expand Up @@ -181,6 +183,7 @@ void test_defaults() {
assertEquals(10000L, config.getQueryPlanCaptureMaxTimeMillis());
assertEquals(10, config.getQueryPlanCaptureMaxCount());
assertThat(config.getLengthCheck()).isEqualTo(LengthCheck.OFF);
assertFalse(config.isIncludeLabelInSql());

config.setLoadModuleInfo(false);
assertFalse(config.isAutoLoadModuleInfo());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
package io.ebeaninternal.server.query;

import io.ebean.CountDistinctOrder;
import io.ebean.Query;
import io.ebean.RawSql;
import io.ebean.RawSqlBuilder;
import io.ebean.*;
import io.ebean.annotation.Platform;
import io.ebean.config.dbplatform.DatabasePlatform;
import io.ebean.config.dbplatform.SqlLimitRequest;
Expand Down Expand Up @@ -52,11 +49,13 @@ final class CQueryBuilder {
private final CQueryDraftSupport draftSupport;
private final DatabasePlatform dbPlatform;
private final boolean selectCountWithColumnAlias;
private final boolean includeLabelInSql;

/**
* Create the SqlGenSelect.
*/
CQueryBuilder(DatabasePlatform dbPlatform, Binder binder, CQueryHistorySupport historySupport, CQueryDraftSupport draftSupport) {
CQueryBuilder(DatabaseBuilder.Settings config, DatabasePlatform dbPlatform, Binder binder, CQueryHistorySupport historySupport, CQueryDraftSupport draftSupport) {
this.includeLabelInSql = config.isIncludeLabelInSql();
this.dbPlatform = dbPlatform;
this.binder = binder;
this.draftSupport = draftSupport;
Expand Down Expand Up @@ -596,7 +595,7 @@ private void appendSelect() {
}

private void appendSelectDistinct() {
sb.append("select ");
sb.append("select ").append(inlineSqlComment());
if (distinct && !countSingleAttribute) {
if (request.isInlineCountDistinct()) {
sb.append("count(");
Expand All @@ -609,6 +608,25 @@ private void appendSelectDistinct() {
}
}

private String inlineSqlComment() {
if (!includeLabelInSql) {
return "";
}
SpiQuery.Type type = query.type();
if (type == SpiQuery.Type.SQ_EX || type == SpiQuery.Type.SQ_EXISTS) {
return "";
}
final var label = query.label();
if (label != null) {
return dbPlatform.inlineSqlComment(label);
}
final var profileLocation = query.profileLocation();
if (profileLocation != null) {
return dbPlatform.inlineSqlComment(profileLocation.label());
}
return "";
}

private void appendFrom() {
if (selectClause == null || !selectClause.startsWith("update")) {
sb.append(" from ");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public CQueryEngine(DatabaseBuilder.Settings config, DatabasePlatform dbPlatform
this.defaultFetchSizeFindList = config.getJdbcFetchSizeFindList();
this.forwardOnlyHintOnFindIterate = dbPlatform.forwardOnlyHintOnFindIterate();
this.historySupport = new CQueryHistorySupport(dbPlatform.historySupport(), asOfTableMapping, config.getAsOfSysPeriod());
this.queryBuilder = new CQueryBuilder(dbPlatform, binder, historySupport, new CQueryDraftSupport(draftTableMap));
this.queryBuilder = new CQueryBuilder(config, dbPlatform, binder, historySupport, new CQueryDraftSupport(draftTableMap));
}

public int forwardOnlyFetchSize() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,9 @@ public boolean isRelaxedMode() {
@Override
public DtoQuery<T> setLabel(String label) {
this.label = label;
if (ormQuery != null) {
ormQuery.setLabel(label);
}
return this;
}

Expand Down
8 changes: 4 additions & 4 deletions ebean-querybean/src/test/java/org/querytest/MyInnerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ void insert_and_find() {

List<String> sql = LoggedSql.stop();
assertThat(sql).hasSize(4);
assertThat(sql.get(0)).contains("select t0.id, t0.one, t0.id, t0.one, t0.description from my_inner t0 where t0.one = ?;");
assertThat(sql.get(1)).contains("select t0.id, t0.one, t0.id, t0.one, t0.description from my_inner t0 where t0.description = ?;");
assertThat(sql.get(2)).contains("select t0.id, t0.one, t0.id, t0.one, t0.description from my_inner t0 where t0.id < ? and t0.description = ?;");
assertThat(sql.get(3)).contains("select t0.id, t0.one, t0.id, t0.one, t0.description from my_inner t0 where t0.id < ? and t0.one > ? and t0.one >= ? and t0.one < ? and t0.one <= ? and t0.id > ?;");
assertThat(sql.get(0)).contains("select /* MyInnerTest.insert_and_find:36 */ t0.id, t0.one, t0.id, t0.one, t0.description from my_inner t0 where t0.one = ?;");
assertThat(sql.get(1)).contains("select /* MyInnerTest.insert_and_find:43 */ t0.id, t0.one, t0.id, t0.one, t0.description from my_inner t0 where t0.description = ?;");
assertThat(sql.get(2)).contains("select /* MyInnerTest.insert_and_find:54 */ t0.id, t0.one, t0.id, t0.one, t0.description from my_inner t0 where t0.id < ? and t0.description = ?;");
assertThat(sql.get(3)).contains("select /* MyInnerTest.insert_and_find:65 */ t0.id, t0.one, t0.id, t0.one, t0.description from my_inner t0 where t0.id < ? and t0.one > ? and t0.one >= ? and t0.one < ? and t0.one <= ? and t0.id > ?;");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ public void testQuery() {
@Test
public void testOrWithExists() {
QCustomer query = Customer.find.typed()
.setLabel("hiLabel")
.alias("_cust")
.or()
.name.eq("Superman")
Expand All @@ -72,12 +73,12 @@ public void testOrWithExists() {
.query()
)
.endOr()
.select(QCustomer.alias().id);
.select(QCustomer.Alias.id);

query.findList();

assertThat(query.getGeneratedSql()).isEqualTo(
"select _cust.id from be_customer _cust where (" +
"select /* hiLabel */ _cust.id from be_customer _cust where (" +
"_cust.name = ? or exists (select 1 from be_contact contact where " +
"contact.first_name = ? and contact.customer_id = _cust.id))"
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ void filterMany() {
.query();

q.findList();
assertThat(q.getGeneratedSql()).isEqualTo("select t0.id, t0.name, t1.id, t1.first_name, t1.last_name from be_customer t0 left join be_contact t1 on t1.customer_id = t0.id where t1.first_name like ? escape'|' and t1.email is not null order by t0.id");
assertThat(q.getGeneratedSql()).isEqualTo("select /* QCustomerTest.filterMany */ t0.id, t0.name, t1.id, t1.first_name, t1.last_name from be_customer t0 left join be_contact t1 on t1.customer_id = t0.id where t1.first_name like ? escape'|' and t1.email is not null order by t0.id");
}

@Test
Expand All @@ -296,7 +296,7 @@ void filterManySingle() {
.query();

q.findList();
assertThat(q.getGeneratedSql()).isEqualTo("select t0.id, t0.name, t1.id, t1.first_name, t1.last_name from be_customer t0 left join be_contact t1 on t1.customer_id = t0.id where t1.first_name like ? escape'|' order by t0.id");
assertThat(q.getGeneratedSql()).isEqualTo("select /* QCustomerTest.filterManySingle */ t0.id, t0.name, t1.id, t1.first_name, t1.last_name from be_customer t0 left join be_contact t1 on t1.customer_id = t0.id where t1.first_name like ? escape'|' order by t0.id");
}

@Test
Expand Down
Loading

0 comments on commit 8b858a0

Please sign in to comment.