Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor for table.definition.datatype.ColumnType #1588

Closed
amorton opened this issue Oct 22, 2024 · 2 comments
Closed

Refactor for table.definition.datatype.ColumnType #1588

amorton opened this issue Oct 22, 2024 · 2 comments
Assignees

Comments

@amorton
Copy link
Contributor

amorton commented Oct 22, 2024

In io.stargate.sgv2.jsonapi.api.model.command.table.definition.datatype.ColumnType

there is this list which duplicates the API names from the APIDataType

  static List<String> getSupportedTypes() {
    return List.of(
        "ascii",
        "bigint",
        "blob",
        "boolean",
        "date",
        "decimal",
        "double",
        "duration",
        "float",
        "inet",
        "int",
        "list",
        "map",
        "set",
        "smallint",
        "text",
        "time",
        "timestamp",
        "tinyint",
        "varint",
        "uuid",
        "vector");
  }

The class then has a switch statement that uses the string constants for "map" etc, these should be reference to the API names on the ApiDataType implementations no duplicated here

then the error at the bottom uses the list from the top as below:

        {
          ColumnType columnType = PrimitiveTypes.fromString(type);
          if (columnType != null) {
            return columnType;
          }
          Map<String, String> errorMessageFormattingValues =
              Map.of(
                  "type",
                  type,
                  "supported_types",
                  "[" + String.join(", ", ColumnType.getSupportedTypes()) + "]");
          throw SchemaException.Code.COLUMN_TYPE_UNSUPPORTED.get(errorMessageFormattingValues);
        }
    }

This looks like the only place are tracking the support for the different types, and I am guessing this for the createTable use

We should add a APISupport type to the ApiDataType type so we can iterate all the types and filter for the level of support we need

we can use that in tests to iterate all of the types and only select the ones that can be used in createTable.

@amorton
Copy link
Contributor Author

amorton commented Oct 23, 2024

Also use of "map" etc string constants in this class should be refactored ComplexApiDataType

@amorton
Copy link
Contributor Author

amorton commented Nov 6, 2024

Refactored , is in the ApiTypeName enum

@amorton amorton closed this as completed Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants