Skip to content

Commit

Permalink
Add calcite system property / adjust test
Browse files Browse the repository at this point in the history
  • Loading branch information
@rorueda
rorueda committed Sep 25, 2024
1 parent 27acfb2 commit 714a7ef
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 18 deletions.
11 changes: 8 additions & 3 deletions core/src/main/java/org/apache/calcite/adapter/enumerable/EnumUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@
import java.util.TimeZone;
import java.util.function.Function;

import static org.apache.calcite.config.CalciteSystemProperty.JOIN_SELECTOR_COMPACT_CODE_THRESHOLD;

import static java.util.Objects.requireNonNull;

/**
Expand Down Expand Up @@ -162,9 +164,7 @@ static Expression joinSelector(JoinRelType joinType, PhysType physType,
final int outputFieldCount = physType.getRowType().getFieldCount();
// If there are many output fields, create the output dynamically so that the code size stays
// below the limit. See CALCITE-3094.
// TODO Set to 100 before merging to master
// Temporarily set to 0, so all the tests are run with the compact code generation
if (outputFieldCount > 0) {
if (shouldGenerateCompactCode(outputFieldCount)) {
return joinSelectorCompact(joinType, physType, inputPhysTypes);
}

Expand Down Expand Up @@ -208,6 +208,11 @@ static Expression joinSelector(JoinRelType joinType, PhysType physType,
parameters);
}

static boolean shouldGenerateCompactCode(int outputFieldCount) {
int compactCodeThreshold = JOIN_SELECTOR_COMPACT_CODE_THRESHOLD.value();
return compactCodeThreshold >= 0 && outputFieldCount >= compactCodeThreshold;
}

static Expression joinSelectorCompact(JoinRelType joinType, PhysType physType,
List<PhysType> inputPhysTypes) {
// A parameter for each input.
Expand Down
17 changes: 17 additions & 0 deletions core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,23 @@ public final class CalciteSystemProperty<T> {
public static final CalciteSystemProperty<Integer> FUNCTION_LEVEL_CACHE_MAX_SIZE =
intProperty("calcite.function.cache.maxSize", 0, v -> v >= 0);

/**
* Minimum numbers of fields in a Join result that will trigger the "compact code generation".
* This feature reduces the risk of running into a compilation error due to the code of a
* dynamically generated method growing beyond the 64KB limit.
*
* <p>Note that the compact code makes use of arraycopy operations when possible,
* instead of using a static array initialization. For joins with a large number of fields
* the resulting code should be faster, but it can be slower for joins with a very small number
* of fields.
*
* <p>The default value is 100, a negative value disables completely the "compact code" feature.
*
* @see org.apache.calcite.adapter.enumerable.EnumUtils
*/
public static final CalciteSystemProperty<Integer> JOIN_SELECTOR_COMPACT_CODE_THRESHOLD =
intProperty("calcite.join.selector.compact.code.threshold", 100);

private static CalciteSystemProperty<Boolean> booleanProperty(String key,
boolean defaultValue) {
// Note that "" -> true (convenient for command-lines flags like '-Dflag')
Expand Down
47 changes: 32 additions & 15 deletions core/src/test/java/org/apache/calcite/test/LargeGeneratedJoinTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
import java.util.function.BiConsumer;
import java.util.function.Function;

import static org.apache.calcite.config.CalciteSystemProperty.JOIN_SELECTOR_COMPACT_CODE_THRESHOLD;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -111,11 +113,26 @@ private static QueryableTable tab(String table, int fieldCount) {
};
}

private static int getBaseTableSize() {
// If compact code generation is turned off, we generate tables that
// will cause the issue. Otherwise, to avoid impacting the test duration,
// we only generate tables wide enough to enable the compact code generation.
int compactCodeThreshold = JOIN_SELECTOR_COMPACT_CODE_THRESHOLD.value();
return compactCodeThreshold < 0 ? 3000 : Math.max(100, compactCodeThreshold);
}

private static int getT0Size() {
return getBaseTableSize();
}
private static int getT1Size() {
return getBaseTableSize() + 1;
}

private static CalciteAssert.AssertQuery assertQuery(String sql) {
Schema rootSchema = new AbstractSchema() {
@Override protected Map<String, Table> getTableMap() {
return ImmutableMap.of("T0", tab("T0", 100),
"T1", tab("T1", 101));
return ImmutableMap.of("T0", tab("T0", getT0Size()),
"T1", tab("T1", getT1Size()));
}
};

Expand All @@ -142,7 +159,7 @@ private static CalciteAssert.AssertQuery assertQuery(String sql) {
query.returns(rs -> {
try {
assertTrue(rs.next());
assertEquals(1 + 100 + 101, rs.getMetaData().getColumnCount());
assertEquals(1 + getT0Size() + getT1Size(), rs.getMetaData().getColumnCount());
long row = 0;
do {
++row;
Expand All @@ -151,11 +168,11 @@ private static CalciteAssert.AssertQuery assertQuery(String sql) {
if (i == 1) {
assertEquals("v0v1", rs.getString(i),
"Error at row: " + row + ", column: " + i);
} else if (i == rs.getMetaData().getColumnCount()) {
assertEquals("v100", rs.getString(i),
} else if (i <= getT0Size() + 1) {
assertEquals("v" + (i - 2), rs.getString(i),
"Error at row: " + row + ", column: " + i);
} else {
assertEquals("v" + ((i - 2) % 100), rs.getString(i),
assertEquals("v" + ((i - 2) - getT0Size()), rs.getString(i),
"Error at row: " + row + ", column: " + i);
}
}
Expand Down Expand Up @@ -184,7 +201,7 @@ private static CalciteAssert.AssertQuery assertQuery(String sql) {
query.returns(rs -> {
try {
assertTrue(rs.next());
assertEquals(1 + 100 + 101, rs.getMetaData().getColumnCount());
assertEquals(1 + getT0Size() + getT1Size(), rs.getMetaData().getColumnCount());
long row = 0;
do {
++row;
Expand All @@ -193,7 +210,7 @@ private static CalciteAssert.AssertQuery assertQuery(String sql) {
if (i == 1) {
assertEquals("v0v1", rs.getString(i),
"Error at row: " + row + ", column: " + i);
} else if (i <= 101) {
} else if (i <= getT0Size() + 1) {
assertEquals("v" + (i - 2), rs.getString(i),
"Error at row: " + row + ", column: " + i);
} else {
Expand Down Expand Up @@ -226,7 +243,7 @@ private static CalciteAssert.AssertQuery assertQuery(String sql) {
query.returns(rs -> {
try {
assertTrue(rs.next());
assertEquals(1 + 100 + 101, rs.getMetaData().getColumnCount());
assertEquals(1 + getT0Size() + getT1Size(), rs.getMetaData().getColumnCount());
long row = 0;
do {
++row;
Expand All @@ -235,11 +252,11 @@ private static CalciteAssert.AssertQuery assertQuery(String sql) {
if (i == 1) {
assertEquals("v0v1", rs.getString(i),
"Error at row: " + row + ", column: " + i);
} else if (i <= 101) {
} else if (i <= getT0Size() + 1) {
assertNull(rs.getString(i),
"Error at row: " + row + ", column: " + i);
} else {
assertEquals("v" + (i - 2 - 100), rs.getString(i),
assertEquals("v" + (i - 2 - getT0Size()), rs.getString(i),
"Error at row: " + row + ", column: " + i);
}
}
Expand Down Expand Up @@ -268,7 +285,7 @@ private static CalciteAssert.AssertQuery assertQuery(String sql) {
query.returns(rs -> {
try {
assertTrue(rs.next());
assertEquals(1 + 1 + 100 + 101, rs.getMetaData().getColumnCount());
assertEquals(1 + 1 + getT0Size() + getT1Size(), rs.getMetaData().getColumnCount());
long row = 0;
do {
++row;
Expand All @@ -281,7 +298,7 @@ private static CalciteAssert.AssertQuery assertQuery(String sql) {
} else if (i == 2) {
assertNull(rs.getString(i),
"Error at row: " + row + ", column: " + i);
} else if (i <= 102) {
} else if (i <= getT0Size() + 2) {
assertEquals("v" + (i - 3), rs.getString(i),
"Error at row: " + row + ", column: " + i);
} else {
Expand All @@ -296,11 +313,11 @@ private static CalciteAssert.AssertQuery assertQuery(String sql) {
} else if (i == 2) {
assertEquals("v0v1", rs.getString(i),
"Error at row: " + row + ", column: " + i);
} else if (i <= 102) {
} else if (i <= getT0Size() + 2) {
assertNull(rs.getString(i),
"Error at row: " + row + ", column: " + i);
} else {
assertEquals("v" + (i - 3 - 100), rs.getString(i),
assertEquals("v" + (i - 3 - getT0Size()), rs.getString(i),
"Error at row: " + row + ", column: " + i);
}
}
Expand Down

0 comments on commit 714a7ef

Please sign in to comment.