apache / gravitino

World's most powerful open data catalog for building a high-performance, geo-distributed and federated metadata lake.
https://gravitino.apache.org
Apache License 2.0
906 stars 292 forks source link

[Bug report] MySQL catalog and Iceberg catalog: create table with Decimal(0,0) column type or add this column type didn't pop error as expected #4115

Closed danhuawang closed 1 month ago

danhuawang commented 1 month ago

Version

main branch

Describe what's wrong

in Mysql catalog, when create a table with Decimal(0,0) type column, it turn out to be created as decimal(10,0) column type in Iceberg catalog, when create a table with Decimal(0,0) type column, table created. in Postgres catalog and Hive catalog, create a table with Decimal(0,0) type column isn't allowed

image image

Error message and/or stacktrace

N/A

How to reproduce

  1. create Mysql catalog, create Iceberg catalog
  2. create table with Decimal(0,0) column type request body
    {
    "name": "test3",
    "comment": "test",
    "columns": [
    {
      "name": "id",
      "type": "integer",
      "comment": "integer column",
      "nullable": true
    },
    {
      "name": "amount",
      "type": "Decimal(0,0)",
      "comment": "Decimal column",
      "nullable": true
    }]
    }

    It shouldn't be allowed to create the table having this column type

Additional context

No response

FANNG1 commented 1 month ago

As decimal(0, 0) is not a valid, maybe we should validate it when creating a decimal type in Gravitino.

FANNG1 commented 1 month ago

@jerryshao @mchades , I prefer do a validate for decimal since decimal(0,0) is meaningless, WDYT?

mchades commented 1 month ago

@jerryshao @mchades , I prefer do a validate for decimal since decimal(0,0) is meaningless, WDYT?

Can you describe how Decima(0,0) behaves in different engines?

FANNG1 commented 1 month ago

@jerryshao @mchades , I prefer do a validate for decimal since decimal(0,0) is meaningless, WDYT?

Can you describe how Decima(0,0) behaves in different engines?

Spark doesn't check precision and scale, while Trino checks

    public static DecimalType createDecimalType(int precision, int scale)
    {
        if (precision <= 0 || precision > MAX_PRECISION) {
            throw new TrinoException(INVALID_FUNCTION_ARGUMENT, format("DECIMAL precision must be in range [1, %d]: %s", MAX_PRECISION, precision));
        }

        if (scale < 0 || scale > precision) {
            throw new TrinoException(INVALID_FUNCTION_ARGUMENT, format("DECIMAL scale must be in range [0, precision (%s)]: %s", precision, scale));
        }

        if (precision <= MAX_SHORT_PRECISION) {
            return ShortDecimalType.getInstance(precision, scale);
        }
        return new LongDecimalType(precision, scale);
    }