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

Alternator: DescribeTable can return spurious RANGE key for GSI #5320

Open
nyh opened this issue Nov 21, 2019 · 1 comment
Open

Alternator: DescribeTable can return spurious RANGE key for GSI #5320

nyh opened this issue Nov 21, 2019 · 1 comment
Labels
area/alternator Alternator related Issues user request
Milestone

Comments

@nyh
Copy link
Contributor

nyh commented Nov 21, 2019

In the current implementation, when DescribeTable wants to list the table's key columns (KeySchema) we don't have the originally requested schema explicitly saved anywhere, so describe_key_schema() attempts to recreate this information from the Scylla table's schema.

But there is one problem with this when describing the KeySchema of a GSI (note that DescribeTable returns the schema of the base table and of all its indexes). The problem is in one specific case: In DynamoDB, one can have a base table with hash key "p", and create a GSI with a different hash key "x" and no range key. In Scylla, we had to, internally, also add "p" as a range key to the materialized view - because a view key must include the base's key columns. This means that with the current implementation, this extra range key "x" is returned by DescribeTable - yet it shouldn't have been. We have an xfailing test case - test_gsi_2_describe_table_schema - to reproduce this case.

@nyh nyh added low area/alternator Alternator related Issues labels Nov 21, 2019
psarna pushed a commit that referenced this issue Nov 21, 2019
One of the fields still missing in DescribeTable's response (Refs #5026)
was the table's schema - KeySchema and AttributeDefinitions.

This patch adds this missing feature, and enables the previously-xfailing
test test_describe_table_schema.

A complication of this patch is that in a table with secondary indexes,
we need to return not just the base table's schema, but also the indexes'
schema. The existing tests did not cover that feature, so we add here
two more tests in test_gsi.py for that.

One of these secondary-index schema tests, test_gsi_2_describe_table_schema,
still fails, because it outputs a range-key which Scylla added to a view
because of its own implementation needs, but wasn't in the user's
definition of the GSI. I opened a separate issue #5320 for that.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
avikivity pushed a commit that referenced this issue Nov 24, 2019
One of the fields still missing in DescribeTable's response (Refs #5026)
was the table's schema - KeySchema and AttributeDefinitions.

This patch adds this missing feature, and enables the previously-xfailing
test test_describe_table_schema.

A complication of this patch is that in a table with secondary indexes,
we need to return not just the base table's schema, but also the indexes'
schema. The existing tests did not cover that feature, so we add here
two more tests in test_gsi.py for that.

One of these secondary-index schema tests, test_gsi_2_describe_table_schema,
still fails, because it outputs a range-key which Scylla added to a view
because of its own implementation needs, but wasn't in the user's
definition of the GSI. I opened a separate issue #5320 for that.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
@slivne slivne added this to the 3.x milestone Jan 2, 2020
@nyh
Copy link
Contributor Author

nyh commented Sep 6, 2022

@bartpeeters commented in #11455 (comment):

It is one of the things that makes Alternator not work with Terraform, after creating the table using Terraform and then applying the plan again it will pickup 'changes' to the index:

- global_secondary_index {
    - hash_key           = "ownerId" -> null
    - non_key_attributes = [] -> null
    - range_key          = "channelId" -> null
    - read_capacity      = 0 -> null
    - write_capacity     = 0 -> null
  }
+ global_secondary_index {
    + hash_key           = "ownerId"
    + name               = "ownerId-index"
    + non_key_attributes = []
    + projection_type    = "ALL"
  }

Terraform index resource code:

global_secondary_index {
  name            = "ownerId-index"
  hash_key        = "ownerId"
  projection_type = "ALL"
}

But I don't think this is the only thing tripping up Terraform, you also don't see the name or projection_type properties on the current table.

@nyh nyh removed the low label Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/alternator Alternator related Issues user request
Projects
None yet
Development

No branches or pull requests

2 participants