Misc review changes

Author: mrotteveel

src/docs/asciidoc/release_notes.adoc

@@ -561,7 +561,7 @@ we recommend to always use `null` for `catalog`
 * `FirebirdConnection`/`FBConnection`
 ** Added method `String getSearchPath()` to obtain the search path as reported by `RBB$GET_CONTEXT('SYSTEM', 'SEARCH_PATH')`, or `null` if schemas are not supported
 ** Added method `List<String> getSearchPatList()` to obtain the search path as a list of unquoted object names, or empty list if schemas are not supported
-** Added methods `setSearchPath(String)` and `setSearchPathList(String...)/(List<String>)` where added to set the search path;
+** Added methods `setSearchPath(String)` and `setSearchPathList(String...)/(List<String>)` to set the search path;
 these methods throw `SQLFeatureNotSupportedException` if schemas are not supported.
 * `StatisticsManager`
 ** `getTableStatistics`

src/main/org/firebirdsql/gds/AbstractVersion.java

@@ -11,6 +11,7 @@
 /**
  * Abstract version for {@code major.minor} version information.
  *
+ * @author Mark Rotteveel
  * @since 7
  */
 public abstract class AbstractVersion implements Comparable<AbstractVersion>, Serializable {

src/main/org/firebirdsql/gds/ServiceRequestBuffer.java

@@ -252,8 +252,8 @@ public interface ServiceRequestBuffer extends ParameterBuffer {
     /**
      * Repeatable buffer item for a table name for operation {@link #STATS_DATA_PAGES}.
      * <p>
-     * For backwards compatibility, it can also be used as a flag, with a space separated list of table names in buffer
-     * item {@link SpbItems#isc_spb_command_line}.
+     * For backwards compatibility, it can also be used as an options flag, with a space separated list of table names
+     * in buffer item {@link SpbItems#isc_spb_command_line}.
      * </p>
      */
     int STATS_TABLE                     = ISCConstants.isc_spb_sts_table;

src/main/org/firebirdsql/gds/ng/OdsVersion.java

@@ -41,7 +41,8 @@ private OdsVersion(int major, int minor) {
      */
     public static OdsVersion of(int major, int minor) {
         if ((major & 0xFFFF) != major || (minor & 0xFFFF) != minor) {
-            throw new IllegalArgumentException("Implementation limit for major or minor exceeded");
+            throw new IllegalArgumentException(
+                    "Implementation limit for major or minor exceeded (must be between 0 and 65535)");
         }
         return ODS_VERSION_CACHE.computeIfAbsent(key(major, minor), ignored -> new OdsVersion(major, minor));
     }

src/main/org/firebirdsql/gds/ng/fields/FieldDescriptor.java

@@ -73,7 +73,8 @@ public final class FieldDescriptor {
     public FieldDescriptor(int position, DatatypeCoder datatypeCoder,
             int type, int subType, int scale, int length,
             String fieldName, String tableAlias,
-            String originalName, String originalSchema, String originalTableName, String ownerName) {
+            String originalName, String originalSchema, String originalTableName,
+            String ownerName) {
         this.position = position;
         this.datatypeCoder = datatypeCoderForType(datatypeCoder, type, subType, scale);
         this.type = type;

src/main/org/firebirdsql/jaybird/parser/ObjectReferenceExtractor.java

@@ -21,6 +21,7 @@
  * the parser again.
  * </p>
  *
+ * @author Mark Rotteveel
  * @since 7
  */
 public final class ObjectReferenceExtractor implements TokenVisitor {
@@ -62,9 +63,9 @@ public void visitToken(Token token, VisitorRegistrar visitorRegistrar) {
                     identifiers.add(new Identifier(oldIdentifier.name(), identifierScope));
                     this.previousTokenWasScopeSpecifierValue = true;
                 } else {
-                    // End of identifier chain, we're no longer interested
+                    // End of identifier chain, we're no longer interested.
                     // This is unexpected, but we're assuming future implementation of % as remainder or modular division
-                    // or some other kind of operator which would end the identifier chain
+                    // or some other kind of operator which would end the identifier chain.
                     visitorRegistrar.removeVisitor(this);
                     return;
                 }
@@ -98,9 +99,8 @@ private boolean isScopeSpecifier(Token token) {
     }
 
     private boolean isScopeSpecifierValue(Token token) {
-        // SCHEMA is a reserved word, PACKAGE is not
-        return token instanceof GenericToken && token.equalsIgnoreCase("PACKAGE")
-                || token instanceof ReservedToken && token.equalsIgnoreCase("SCHEMA");
+        // NOTE: SCHEMA is a reserved word, PACKAGE is not
+        return token.equalsIgnoreCase("PACKAGE") || token.equalsIgnoreCase("SCHEMA");
     }
 
     @Override

src/main/org/firebirdsql/jaybird/parser/SearchPathExtractor.java

@@ -15,6 +15,9 @@
  * This visitor is written for the needs of {@link SearchPathHelper#parseSearchPath(String)}, and
  * may not be generally usable.
  * </p>
+ *
+ * @author Mark Rotteveel
+ * @since 7
  */
 public final class SearchPathExtractor implements TokenVisitor {
 
@@ -37,7 +40,8 @@ private void extractIdentifier(Token token, VisitorRegistrar visitorRegistrar) {
             if (token instanceof QuotedIdentifierToken quotedIdentifier) {
                 identifiers.add(quotedIdentifier.name());
             } else if (token instanceof GenericToken identifier && identifier.isValidIdentifier()) {
-                // Firebird returns the search path with quoted identifiers, but this offers extra flexibility if needed
+                // Firebird returns the search path with quoted identifiers, but this way we can also parse unquoted
+                // values (e.g. user-provided)
                 identifiers.add(identifier.text().toUpperCase(Locale.ROOT));
             } else {
                 // Unexpected token, end parsing
@@ -68,7 +72,7 @@ public void complete(VisitorRegistrar visitorRegistrar) {
     }
 
     /**
-     * The extract search path list, or empty if not parsed or if the parsed text was not a valid search path list.
+     * The extracted search path list, or empty if not parsed or if the parsed text was not a valid search path list.
      *
      * @return immutable list of unquoted search path entries
      */

src/main/org/firebirdsql/jaybird/parser/StatementIdentification.java

@@ -43,7 +43,8 @@ public LocalStatementType getStatementType() {
      * It reports the name normalized to its metadata storage representation.
      * </p>
      *
-     * @return Schema, {@code null} if the table was not qualified, or for {@code SELECT} and other non-DML statements
+     * @return schema, {@code null} if the table was not qualified, or for {@code SELECT} and other non-DML statements
+     * @since 7
      */
     public @Nullable String getSchema() {
         return schema;
@@ -55,7 +56,7 @@ public LocalStatementType getStatementType() {
      * It reports the name normalized to its metadata storage representation.
      * </p>
      *
-     * @return Table name, {@code null} for {@code SELECT} and other non-DML statements
+     * @return table name, {@code null} for {@code SELECT} and other non-DML statements
      */
     public @Nullable String getTableName() {
         return tableName;
@@ -78,13 +79,10 @@ public boolean returningClauseDetected() {
      */
     private static @Nullable String normalizeObjectName(@Nullable Token objectToken) {
         if (objectToken == null) return null;
-        String objectName = objectToken.text().trim();
-        if (objectName.length() > 2
-                && objectName.charAt(0) == '"'
-                && objectName.charAt(objectName.length() - 1) == '"') {
-            return objectName.substring(1, objectName.length() - 1).replace("\"\"", "\"");
+        if (objectToken instanceof QuotedIdentifierToken quotedIdentifier) {
+            return quotedIdentifier.name();
         }
-        return objectName.toUpperCase(Locale.ROOT);
+        return objectToken.text().toUpperCase(Locale.ROOT);
     }
 
 }

src/main/org/firebirdsql/jaybird/parser/UnexpectedTokenException.java

@@ -15,6 +15,7 @@
  * it from methods that a user calls to obtain the result of a token visitor.
  * </p>
  *
+ * @author Mark Rotteveel
  * @since 7
  */
 public class UnexpectedTokenException extends IllegalStateException {

src/main/org/firebirdsql/jaybird/props/DatabaseConnectionProperties.java

@@ -827,6 +827,7 @@ default void setMaxBlobCacheSize(int maxBlobCacheSize) {
     /**
      * @return the initial search path of the connection, {@code null} if the server default search path is used
      * @see #setSearchPath(String)
+     * @since 7
      */
     default String getSearchPath() {
         return getProperty(PropertyNames.searchPath);
@@ -845,6 +846,7 @@ default String getSearchPath() {
      *
      * @param searchPath
      *         list of comma-separated schema names
+     * @since 7
      */
     default void setSearchPath(String searchPath) {
         setProperty(PropertyNames.searchPath, searchPath);