Skip to content

Commit

Permalink
#813 FBDatabaseMetaData.getIndexInfo error on Firebird 5.0 with a Fir…
Browse files Browse the repository at this point in the history
…ebird 4.0 database

Resolves: #814
  • Loading branch information
mrotteveel committed Jul 26, 2024
1 parent 2a20377 commit 27c3d39
Show file tree
Hide file tree
Showing 9 changed files with 168 additions and 7 deletions.
9 changes: 8 additions & 1 deletion src/main/org/firebirdsql/gds/ng/OdsVersion.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
* @author Mark Rotteveel
* @since 6
*/
public final class OdsVersion {
public final class OdsVersion implements Comparable<OdsVersion> {

private static final Map<Integer, OdsVersion> ODS_VERSION_CACHE = new ConcurrentHashMap<>();

Expand Down Expand Up @@ -126,4 +126,11 @@ public String toString() {
return major + "." + minor;
}

@Override
public int compareTo(OdsVersion o) {
int majorDiff = Integer.compare(this.major, o.major);
if (majorDiff != 0) return majorDiff;
return Integer.compare(this.minor, o.minor);
}

}
7 changes: 7 additions & 0 deletions src/main/org/firebirdsql/jdbc/DbMetadataMediator.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.firebirdsql.gds.impl.GDSType;
import org.firebirdsql.gds.ng.DatatypeCoder;
import org.firebirdsql.gds.ng.DefaultDatatypeCoder;
import org.firebirdsql.gds.ng.OdsVersion;
import org.firebirdsql.gds.ng.fields.RowDescriptorBuilder;
import org.firebirdsql.util.FirebirdSupportInfo;
import org.firebirdsql.util.InternalApi;
Expand Down Expand Up @@ -96,6 +97,12 @@ public static RowDescriptorBuilder newRowDescriptorBuilder(int size) {
*/
public abstract Collection<String> getClientInfoPropertyNames();

/**
* @return the ODS version of the database
* @since 6
*/
public abstract OdsVersion getOdsVersion();

/**
* Holder class for query text and parameters.
*/
Expand Down
7 changes: 7 additions & 0 deletions src/main/org/firebirdsql/jdbc/FBDatabaseMetaData.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.firebirdsql.gds.impl.GDSHelper;
import org.firebirdsql.gds.impl.GDSType;
import org.firebirdsql.gds.ng.LockCloseable;
import org.firebirdsql.gds.ng.OdsVersion;
import org.firebirdsql.jaybird.Version;
import org.firebirdsql.jdbc.InternalTransactionCoordinator.MetaDataTransactionCoordinator;
import org.firebirdsql.jdbc.escape.FBEscapedFunctionHelper;
Expand Down Expand Up @@ -2037,5 +2038,11 @@ public Collection<String> getClientInfoPropertyNames() {
}
return emptyList();
}

@Override
public OdsVersion getOdsVersion() {
return gdsHelper.getCurrentDatabase().getOdsVersion();
}

}
}
10 changes: 7 additions & 3 deletions src/main/org/firebirdsql/jdbc/metadata/GetIndexInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@
*/
package org.firebirdsql.jdbc.metadata;

import org.firebirdsql.gds.ng.OdsVersion;
import org.firebirdsql.gds.ng.fields.RowDescriptor;
import org.firebirdsql.gds.ng.fields.RowValue;
import org.firebirdsql.jdbc.DbMetadataMediator;
import org.firebirdsql.jdbc.DbMetadataMediator.MetadataQuery;
import org.firebirdsql.util.FirebirdSupportInfo;

import java.sql.DatabaseMetaData;
import java.sql.ResultSet;
Expand Down Expand Up @@ -115,10 +115,14 @@ RowValue createMetadataRow(ResultSet rs, RowValueBuilder valueBuilder) throws SQ
return valueBuilder.toRowValue(false);
}

/**
* The ODS of a Firebird 5.0 database.
*/
private static final OdsVersion ODS_13_1 = OdsVersion.of(13, 1);

public static GetIndexInfo create(DbMetadataMediator mediator) {
FirebirdSupportInfo firebirdSupportInfo = mediator.getFirebirdSupportInfo();
// NOTE: Indirection through static method prevents unnecessary classloading
if (firebirdSupportInfo.isVersionEqualOrAbove(5)) {
if (mediator.getOdsVersion().compareTo(ODS_13_1) >= 0) {
return FB5.createInstance(mediator);
} else {
return FB2_5.createInstance(mediator);
Expand Down
10 changes: 7 additions & 3 deletions src/main/org/firebirdsql/jdbc/metadata/GetTables.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@
*/
package org.firebirdsql.jdbc.metadata;

import org.firebirdsql.gds.ng.OdsVersion;
import org.firebirdsql.gds.ng.fields.RowDescriptor;
import org.firebirdsql.gds.ng.fields.RowValue;
import org.firebirdsql.jdbc.DbMetadataMediator;
import org.firebirdsql.jdbc.DbMetadataMediator.MetadataQuery;
import org.firebirdsql.jdbc.FBResultSet;
import org.firebirdsql.util.FirebirdSupportInfo;
import org.firebirdsql.util.InternalApi;

import java.sql.ResultSet;
Expand Down Expand Up @@ -143,10 +143,14 @@ private Set<String> toTypesSet(String[] types) {
*/
abstract Set<String> allTableTypes();

/**
* The ODS of a Firebird 2.5 database.
*/
private static final OdsVersion ODS_11_2 = OdsVersion.of(11, 2);

public static GetTables create(DbMetadataMediator mediator) {
FirebirdSupportInfo firebirdSupportInfo = mediator.getFirebirdSupportInfo();
// NOTE: Indirection through static method prevents unnecessary classloading
if (firebirdSupportInfo.isVersionEqualOrAbove(2, 5)) {
if (mediator.getOdsVersion().compareTo(ODS_11_2) >= 0) {
return FB2_5.createInstance(mediator);
} else {
return FB2_1.createInstance(mediator);
Expand Down
1 change: 1 addition & 0 deletions src/main/org/firebirdsql/util/FirebirdSupportInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,7 @@ public boolean supportsOds(int major, int minor) {
case 13 -> switch (minor) {
case 0 -> isVersionEqualOrAbove(4) && isVersionBelow(7);
case 1 -> isVersionEqualOrAbove(5) && isVersionBelow(7);
// TODO Might change before release of Firebird 6
case 2 -> isVersionEqualOrAbove(6) && isVersionBelow(7);
default -> false;
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* Firebird Open Source JDBC Driver
*
* Distributable under LGPL license.
* You may obtain a copy of the License at http://www.gnu.org/copyleft/lgpl.html
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* LGPL License for more details.
*
* This file was created by members of the firebird development team.
* All individual contributions remain the Copyright (C) of those
* individuals. Contributors to this file are either listed here or
* can be obtained from a source control history command.
*
* All rights reserved.
*/
package org.firebirdsql.common.matchers;

import org.hamcrest.Matcher;
import org.hamcrest.Matchers;

/**
* Factory for matchers to assert comparable values.
*
* @author Mark Rotteveel
* @since 6
*/
public final class ComparableMatcherFactory {

private ComparableMatcherFactory() {
// no instances
}

/**
* Creates a {@link Comparable} matcher for the specified {@code comparison}.
*
* @param comparison
* comparison to assert ({@code <}, {@code <=}, {@code ==}, {@code >=}, {@code >}, or {@code lessThan},
* {@code lessThanOrEqualTo}, {@code equalTo}, {@code greaterThanOrEqualTo}, {@code greaterThan})
* @param operand
* operand to compare against
* @param <T>
* type parameter
* @return matcher to assert the comparison
* @throws IllegalArgumentException
* for an unsupported value for {@code comparison}
*/
public static <T extends Comparable<T>> Matcher<T> compares(String comparison, T operand) {
return switch (comparison) {
case "<", "lessThan" -> Matchers.lessThan(operand);
case "<=", "lessThanOrEqualTo" -> Matchers.lessThanOrEqualTo(operand);
case "==", "equalTo" -> Matchers.comparesEqualTo(operand);
case ">=", "greaterThanOrEqualTo" -> Matchers.greaterThanOrEqualTo(operand);
case ">", "greaterThan" -> Matchers.greaterThan(operand);
default -> throw new IllegalArgumentException("Unsupported comparison: " + comparison);
};
}
}
17 changes: 17 additions & 0 deletions src/test/org/firebirdsql/gds/ng/OdsVersionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import java.lang.invoke.MethodType;
import java.util.stream.Stream;

import static org.firebirdsql.common.matchers.ComparableMatcherFactory.compares;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;

/**
Expand Down Expand Up @@ -92,6 +94,21 @@ static Stream<Arguments> keyStriping() {
//@formatting:one
}

@ParameterizedTest
@CsvSource(useHeadersInDisplayName = true, textBlock = """
op1Major, op1Minor, expectedComparison, op2Major, op2Minor
10, 0, ==, 10, 0
10, 0, <, 11, 0
11, 0, >, 10, 0
11, 0, <, 11, 1
11, 1, >, 11, 0
11, 2, <, 13, 1
13, 1, >, 11, 2
""")
void compareTo(int op1Major, int op1Minor, String expectedComparison, int op2Major, int op2Minor) {
assertThat(OdsVersion.of(op1Major, op1Minor), compares(expectedComparison, OdsVersion.of(op2Major, op2Minor)));
}

private static String keyAsString(int key) {
return Long.toString(key & 0x0FFFFFFFFL, 2);
}
Expand Down
54 changes: 54 additions & 0 deletions src/test/org/firebirdsql/jdbc/FBDatabaseMetaDataIndexInfoTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,36 @@
*/
package org.firebirdsql.jdbc;

import org.firebirdsql.common.extension.RunEnvironmentExtension;
import org.firebirdsql.common.extension.UsesDatabaseExtension;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.junit.jupiter.api.io.TempDir;
import org.opentest4j.TestAbortedException;

import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.sql.Connection;
import java.sql.DatabaseMetaData;
import java.sql.DriverManager;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.*;

import static org.firebirdsql.common.FBTestProperties.getConnectionViaDriverManager;
import static org.firebirdsql.common.FBTestProperties.getDefaultPropertiesForConnection;
import static org.firebirdsql.common.FBTestProperties.getDefaultSupportInfo;
import static org.firebirdsql.common.FBTestProperties.getUrl;
import static org.firebirdsql.common.JdbcResourceHelper.closeQuietly;
import static org.firebirdsql.common.matchers.MatcherAssume.assumeThat;
import static org.hamcrest.Matchers.equalTo;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assumptions.assumeTrue;

/**
* Tests for {@link FBDatabaseMetaData} for index info related metadata.
Expand Down Expand Up @@ -224,6 +238,46 @@ void testIndexInfo_table2_unique() throws Exception {
ResultSet indexInfo = dbmd.getIndexInfo(null, null, "INDEX_TEST_TABLE_2", true, false);
validate(indexInfo, expectedIndexInfo);
}

/**
* Test index info retrieval on Firebird 5.0, with a Firebird 4.0 database file.
* <p>
* Rationale: see <a href="https://github.com/FirebirdSQL/jaybird/issues/813">jaybird#813</a>.
* </p>
* <p>
* This test is machine specific (or at least, environment-specific), as it requires a Firebird database with
* the path {@code E:\DB\FB4\FB4TESTDATABASE.FDB}.
* </p>
*/
@Test
void indexInfoOfdOds13_0DbWithFirebirdSupportingPartialIndex(@TempDir Path tempDir) throws Exception {
assumeTrue(getDefaultSupportInfo().supportsPartialIndices(), "Test requires partial index support");
assumeTrue(getDefaultSupportInfo().supportsOds(13, 0), "Test requires ODS 13.0 support (no partial indexes)");
assumeTrue(RunEnvironmentExtension.EnvironmentRequirement.DB_LOCAL_FS.isMet(), "Requires DB on local file system");

Path fb4DbPath;
try {
fb4DbPath = Paths.get("E:/DB/FB4/FB4TESTDATABASE.FDB");
} catch (InvalidPathException e) {
throw new TestAbortedException("Database path is invalid on this system", e);
}
assumeTrue(Files.exists(fb4DbPath), "Expected database does not exist");

Path testDb = tempDir.resolve("tempdb.fdb").toAbsolutePath();
Files.copy(fb4DbPath, testDb);

try (var connection = DriverManager.getConnection(
getUrl(testDb.toString()), getDefaultPropertiesForConnection())) {
var dbmd = connection.getMetaData().unwrap(FirebirdDatabaseMetaData.class);
assumeThat("Unexpected ODS major", dbmd.getOdsMajorVersion(), equalTo(13));
assumeThat("Unexpected ODS minor", dbmd.getOdsMinorVersion(), equalTo(0));

try (ResultSet indexInfo = assertDoesNotThrow(
() -> dbmd.getIndexInfo(null, null, "doesnotexist", false, true))) {
getIndexInfoDefinition.validateResultSetColumns(indexInfo);
}
}
}

// TODO Add tests with quoted identifiers

Expand Down

0 comments on commit 27c3d39

Please sign in to comment.