Skip to content

Commit 1eba621

Browse files
guydavenportguydavenport
authored
Merge pull request #59 from plantbreeding/develop
Release 0.57
2 parents 49695fa + c16d244 commit 1eba621

17 files changed

+81
-68
lines changed

‎java/core/src/main/java/org/brapi/schematools/core/sql/ANSICreateTableDDLGenerator.java‎

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
import static org.brapi.schematools.core.response.Response.fail;
2323
import static org.brapi.schematools.core.response.Response.success;
2424
import static org.brapi.schematools.core.utils.BrAPITypeUtils.unwrapType;
25-
import static org.brapi.schematools.core.utils.StringUtils.escapeSingleSQLQuotes;
25+
import static org.brapi.schematools.core.utils.StringUtils.escapeQuotes;
2626
import static org.brapi.schematools.core.utils.StringUtils.removeCarriageReturns;
2727
import static org.brapi.schematools.core.utils.StringUtils.toPlural;
2828
import static org.brapi.schematools.core.utils.StringUtils.toSentenceCase;
@@ -111,7 +111,7 @@ private Response<String> createTableDefinitionStart(String tableName) {
111111
appendNewLine(builder) ;
112112

113113
if (options.isAddingDropTable()) {
114-
builder.append("DROP TABLE ");
114+
builder.append("DROP TABLE IF EXISTS ");
115115
builder.append(tableName);
116116
builder.append("; ");
117117
appendNewLine(builder) ;
@@ -140,24 +140,32 @@ private Response<String> createTableDefinitionEnd(String tableName, String descr
140140
appendNewLine(builder) ;
141141
builder.append(") ");
142142

143+
if (tableUsing != null) {
144+
appendNewLine(builder) ;
145+
builder.append("USING ");
146+
builder.append(tableUsing);
147+
}
148+
143149
if (options.isClustering()) {
144150

145-
if (!clusterColumns.isEmpty()) {
151+
List<String> columns = clusterColumns;
152+
153+
if (clusterColumns.size() > 4) {
154+
log.warn("Clustering on more than 4 columns is not supported in many SQL dialects, table {} has {} clustering columns. Removing extra ones. ", tableName, clusterColumns.size());
155+
156+
columns = clusterColumns.subList(0, 4) ;
157+
}
158+
159+
if (!columns.isEmpty()) {
146160
appendNewLine(builder) ;
147161
builder.append("CLUSTER BY (");
148-
builder.append(String.join(",", clusterColumns));
162+
builder.append(String.join(",", columns));
149163
builder.append(")");
150164
} else {
151165
log.warn("No clustering columns found for table {}", tableName);
152166
}
153167
}
154168

155-
if (tableUsing != null) {
156-
appendNewLine(builder) ;
157-
builder.append("USING ");
158-
builder.append(tableUsing);
159-
}
160-
161169
if (tableProperties != null && !tableProperties.isEmpty()) {
162170
appendNewLine(builder) ;
163171
builder.append("TBLPROPERTIES (");
@@ -169,7 +177,7 @@ private Response<String> createTableDefinitionEnd(String tableName, String descr
169177
appendNewLine(builder) ;
170178
builder.append("COMMENT '");
171179

172-
builder.append(removeCarriageReturns(escapeSingleSQLQuotes(description)));
180+
builder.append(removeCarriageReturns(escapeQuotes(description)));
173181

174182
builder.append("'");
175183
}
@@ -217,9 +225,9 @@ private String createTableName(BrAPIObjectType brAPIObjectType) {
217225

218226
private String getTableDescription(BrAPIObjectType brAPIObjectType) {
219227
if (brAPIObjectType.getDescription() != null) {
220-
return removeCarriageReturns(escapeSingleSQLQuotes(brAPIObjectType.getDescription()));
228+
return removeCarriageReturns(escapeQuotes(brAPIObjectType.getDescription()));
221229
} else {
222-
return removeCarriageReturns(escapeSingleSQLQuotes(options.getDescriptionFor(brAPIObjectType)));
230+
return removeCarriageReturns(escapeQuotes(options.getDescriptionFor(brAPIObjectType)));
223231
}
224232
}
225233

@@ -229,12 +237,14 @@ private List<String> findClusterColumns(BrAPIObjectType brAPIObjectType) {
229237

230238
options.getProperties().getLinkPropertiesFor(brAPIObjectType).forEach(brAPIObjectProperty -> clusterColumns.add(brAPIObjectProperty.getName()));
231239

232-
for (BrAPIObjectProperty brAPIObjectProperty : brAPIObjectType.getProperties()) {
240+
if (options.isClusteringForeignKeys()) {
241+
for (BrAPIObjectProperty brAPIObjectProperty : brAPIObjectType.getProperties()) {
233242

234-
BrAPIType dereferenceType = brAPIClassCache.dereferenceType(brAPIObjectProperty.getType());
243+
BrAPIType dereferenceType = brAPIClassCache.dereferenceType(brAPIObjectProperty.getType());
235244

236-
if (dereferenceType instanceof BrAPIObjectType brAPIObjectPropertyObjectType && ID.equals(getLinkPropertiesFor(brAPIObjectType, brAPIObjectProperty, dereferenceType))) {
237-
options.getProperties().getLinkPropertiesFor(brAPIObjectPropertyObjectType).forEach(linkProperty -> clusterColumns.add(linkProperty.getName()));
245+
if (dereferenceType instanceof BrAPIObjectType brAPIObjectPropertyObjectType && ID.equals(getLinkPropertiesFor(brAPIObjectType, brAPIObjectProperty, dereferenceType))) {
246+
options.getProperties().getLinkPropertiesFor(brAPIObjectPropertyObjectType).forEach(linkProperty -> clusterColumns.add(linkProperty.getName()));
247+
}
238248
}
239249
}
240250

@@ -471,9 +481,9 @@ private Response<String> addColumnComment(BrAPIObjectType brAPIObjectType, BrAPI
471481
builder.append(" COMMENT '");
472482

473483
if (property.getDescription() != null) {
474-
builder.append(removeCarriageReturns(escapeSingleSQLQuotes(property.getDescription())));
484+
builder.append(removeCarriageReturns(escapeQuotes(property.getDescription())));
475485
} else {
476-
builder.append(removeCarriageReturns(escapeSingleSQLQuotes(options.getProperties().getDescriptionFor(brAPIObjectType, property))));
486+
builder.append(removeCarriageReturns(escapeQuotes(options.getProperties().getDescriptionFor(brAPIObjectType, property))));
477487
}
478488

479489
builder.append("'");

‎java/core/src/main/java/org/brapi/schematools/core/sql/options/SQLGeneratorOptions.java‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ public class SQLGeneratorOptions extends AbstractMainGeneratorOptions {
4848
private Boolean generateLinkTables;
4949
private Boolean snakeCaseTableNames;
5050
private Boolean pluralTableNames;
51+
private Boolean clusterForeignKeys;
5152

5253
/**
5354
* Load the default options
@@ -179,6 +180,10 @@ public SQLGeneratorOptions override(SQLGeneratorOptions overrideOptions) {
179180
pluralTableNames = overrideOptions.pluralTableNames;
180181
}
181182

183+
if (overrideOptions.clusterForeignKeys != null) {
184+
clusterForeignKeys = overrideOptions.clusterForeignKeys;
185+
}
186+
182187
return this;
183188
}
184189

@@ -301,10 +306,19 @@ public boolean isUsingSnakeCaseTableNames() {
301306
/**
302307
* Determines if the Generator should use plural table names for entity tables
303308
*
304-
* @return {@code true} if the Generator should use plural table names for /** tables, {@code false} otherwise
309+
* @return {@code true} if the Generator should use plural table names for tables, {@code false} otherwise
305310
*/
306311
@JsonIgnore
307312
public boolean isUsingPluralTableNames() {
308313
return pluralTableNames != null && pluralTableNames ;
309314
}
315+
316+
/**
317+
* Determines if the Generator should cluster key for foreign keys. If there are more than 4 cluster keys, the extra ones will be ignored
318+
*
319+
* @return {@code true} if the Generator should cluster key for foreign keys, {@code false} otherwise
320+
*/
321+
public boolean isClusteringForeignKeys() {
322+
return clusterForeignKeys != null && clusterForeignKeys ;
323+
}
310324
}

‎java/core/src/main/java/org/brapi/schematools/core/utils/StringUtils.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -377,11 +377,7 @@ public static String format(String format, Map<String, Object> parameters) {
377377
}
378378

379379
public static String escapeQuotes(String inputString) {
380-
return inputString.replaceAll("\"", "\"").replaceAll("'", "''") ;
381-
}
382-
383-
public static String escapeSingleSQLQuotes(String inputString) {
384-
return inputString.replaceAll("'", "''") ;
380+
return inputString.replaceAll("\"", "\\\"").replaceAll("'", "\\\\'") ;
385381
}
386382

387383
public static String escapeSpecialCharacters(String inputString) {

‎java/core/src/main/resources/sql-options.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ descriptionFormat: TODO Description for %s
99
generateLinkTables : true
1010
snakeCaseTableNames : false
1111
pluralTableNames : true
12+
clusterForeignKeys : false
1213
generateFor:
1314
AlleleMatrix: false
1415
MarkerPosition: false

‎java/core/src/test/java/org/brapi/schematools/core/utils/StringUtilsTest.java‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -186,17 +186,9 @@ void isJSONEqual() {
186186

187187
@Test
188188
void testEscapeQuotes() {
189-
assertEquals("\\\" '' SimpleText123 ", StringUtils.escapeQuotes("\\\" ' SimpleText123 "));
189+
assertEquals("\\\" \\' SimpleText123 ", StringUtils.escapeQuotes("\\\" ' SimpleText123 "));
190190
assertEquals(
191-
"`levelOrder` defines where that level exists in the hierarchy of levels. `levelOrder`''s lower numbers \nare at the top of the hierarchy (ie field -> 1) and higher numbers are at the bottom of the hierarchy (ie plant -> 9). \n\nFor more information on Observation Levels, please review the <a target=\"_blank\" href=\"https://wiki.brapi.org/index.php/Observation_Levels\">Observation Levels documentation</a>. ",
192-
StringUtils.escapeQuotes("`levelOrder` defines where that level exists in the hierarchy of levels. `levelOrder`'s lower numbers \nare at the top of the hierarchy (ie field -> 1) and higher numbers are at the bottom of the hierarchy (ie plant -> 9). \n\nFor more information on Observation Levels, please review the <a target=\"_blank\" href=\"https://wiki.brapi.org/index.php/Observation_Levels\">Observation Levels documentation</a>. ")) ;
193-
}
194-
195-
@Test
196-
void testEscapeSingleSQLQuotes() {
197-
assertEquals("'' SimpleText123 ", StringUtils.escapeSingleSQLQuotes("' SimpleText123 "));
198-
assertEquals(
199-
"`levelOrder` defines where that level exists in the hierarchy of levels. `levelOrder`''s lower numbers \nare at the top of the hierarchy (ie field -> 1) and higher numbers are at the bottom of the hierarchy (ie plant -> 9). \n\nFor more information on Observation Levels, please review the <a target=\"_blank\" href=\"https://wiki.brapi.org/index.php/Observation_Levels\">Observation Levels documentation</a>. ",
191+
"`levelOrder` defines where that level exists in the hierarchy of levels. `levelOrder`\\'s lower numbers \nare at the top of the hierarchy (ie field -> 1) and higher numbers are at the bottom of the hierarchy (ie plant -> 9). \n\nFor more information on Observation Levels, please review the <a target=\"_blank\" href=\"https://wiki.brapi.org/index.php/Observation_Levels\">Observation Levels documentation</a>. ",
200192
StringUtils.escapeQuotes("`levelOrder` defines where that level exists in the hierarchy of levels. `levelOrder`'s lower numbers \nare at the top of the hierarchy (ie field -> 1) and higher numbers are at the bottom of the hierarchy (ie plant -> 9). \n\nFor more information on Observation Levels, please review the <a target=\"_blank\" href=\"https://wiki.brapi.org/index.php/Observation_Levels\">Observation Levels documentation</a>. ")) ;
201193
}
202194

‎java/core/src/test/resources/SQLGenerator/ANSI/Study-clustering-and-table-properties.sql‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ CREATE TABLE brapi_Studies (
3939
experimentalDesign
4040
STRUCT<
4141
PUI STRING COMMENT 'MIAPPE V1.1 (DM-23) Type of experimental design - Type of experimental design of the study, in the form of an accession number from the Crop Ontology.',
42-
description STRING COMMENT 'MIAPPE V1.1 (DM-22) Description of the experimental design - Short description of the experimental design, possibly including statistical design. In specific cases, e.g. legacy datasets or data computed from several studies, the experimental design can be "unknown"/"NA", "aggregated/reduced data", or simply ''none''.'
42+
description STRING COMMENT 'MIAPPE V1.1 (DM-22) Description of the experimental design - Short description of the experimental design, possibly including statistical design. In specific cases, e.g. legacy datasets or data computed from several studies, the experimental design can be "unknown"/"NA", "aggregated/reduced data", or simply \'none\'.'
4343
> COMMENT 'The experimental and statistical design full description plus a category PUI taken from crop research ontology or agronomy ontology',
4444
externalReferences
4545
ARRAY<
@@ -66,7 +66,7 @@ CREATE TABLE brapi_Studies (
6666
ARRAY<
6767
STRUCT<
6868
levelName STRING COMMENT 'A name for this level **Standard Level Names: study, field, entry, rep, block, sub-block, plot, sub-plot, plant, pot, sample** For more information on Observation Levels, please review the <a target="_blank" href="https://wiki.brapi.org/index.php/Observation_Levels">Observation Levels documentation</a>. ',
69-
levelOrder INT COMMENT '`levelOrder` defines where that level exists in the hierarchy of levels. `levelOrder`''s lower numbers are at the top of the hierarchy (ie field -> 1) and higher numbers are at the bottom of the hierarchy (ie plant -> 9). For more information on Observation Levels, please review the <a target="_blank" href="https://wiki.brapi.org/index.php/Observation_Levels">Observation Levels documentation</a>. '
69+
levelOrder INT COMMENT '`levelOrder` defines where that level exists in the hierarchy of levels. `levelOrder`\'s lower numbers are at the top of the hierarchy (ie field -> 1) and higher numbers are at the bottom of the hierarchy (ie plant -> 9). For more information on Observation Levels, please review the <a target="_blank" href="https://wiki.brapi.org/index.php/Observation_Levels">Observation Levels documentation</a>. '
7070
>
7171
> COMMENT 'Observation levels indicate the granularity level at which the measurements are taken. `levelName` defines the level, `levelOrder` defines where that level exists in the hierarchy of levels. `levelOrder`s lower numbers are at the top of the hierarchy (ie field > 0) and higher numbers are at the bottom of the hierarchy (ie plant > 6). **Standard Level Names: study, field, entry, rep, block, sub-block, plot, sub-plot, plant, pot, sample** For more information on Observation Levels, please review the <a target="_blank" href="https://wiki.brapi.org/index.php/Observation_Levels">Observation Levels documentation</a>. ',
7272
observationUnitsDescription STRING COMMENT 'MIAPPE V1.1 (DM-25) Observation unit description - General description of the observation units in the study.',
@@ -83,7 +83,7 @@ CREATE TABLE brapi_Studies (
8383
trialName STRING COMMENT 'The human readable name of a trial MIAPPE V1.1 (DM-3) Investigation title - Human-readable string summarising the investigation.',
8484
trialPUI STRING COMMENT 'A permanent identifier for a trial. Could be DOI or other URI formatted identifier.'
8585
)
86-
CLUSTER BY (studyDbId,studyName,studyPUI,locationDbId,locationName,trialDbId,trialName,trialPUI)
86+
CLUSTER BY (studyDbId,studyName,studyPUI)
8787
TBLPROPERTIES ('delta.enableChangeDataFeed' = true)
8888
COMMENT 'A Study represents an experiment that has taken place at a single location. The Study contains metadata about the parameters and design of the experiment. It can also be used to group results and data sets generated from the experiment. A Trial can represent a collection of one or more Studies.';
8989

@@ -113,6 +113,6 @@ CREATE TABLE brapi_ObservationVariableByStudy (
113113
studyName STRING COMMENT 'The human readable name for a study MIAPPE V1.1 (DM-12) Study title - Human-readable text summarising the study',
114114
studyPUI STRING COMMENT 'A permanent unique identifier associated with this study data. For example, a URI or DOI'
115115
)
116-
CLUSTER BY (observationVariableDbId,observationVariableName,observationVariablePUI,studyDbId,studyName,studyPUI)
116+
CLUSTER BY (observationVariableDbId,observationVariableName,observationVariablePUI,studyDbId)
117117
TBLPROPERTIES ('delta.enableChangeDataFeed' = true)
118118
COMMENT 'Link table for Study to ObservationVariable on property observationVariables';

0 commit comments

Comments
 (0)