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

Check for shard key or sort key before creating the temporary command #24

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jvelezmagic
Copy link

Summary:
This PR addresses a compatibility issue in Singlestore for incremental models using shard_key or sort_key. Previously, attempting to use these keys would fail due to an incorrect assumption that temporary tables are rowstore, which is not supported by Singlestore for CLUSTERED COLUMNAR indexes.

Details:

  • Problem: Models with shard_key or sort_key couldn't be deployed as incremental tables because the system tried to create them as rowstore temporary tables, conflicting with Singlestore's requirements.
  • Solution: We've updated the logic to dynamically set the storage type for temporary tables. If a model has shard_key or sort_key, it's created as a "temporary" table (compatible with these keys), otherwise, it defaults to "rowstore temporary".
  • Benefits: This update ensures that all incremental models, regardless of their key configurations, can be deployed successfully in Singlestore.

@okramarenko
Copy link
Contributor

Hi @jvelezmagic! Thank you for bringing this to our attention. In SingleStore, the SORT KEY()is not allowed when using CREATE ROWSTORE TABLE. However, there is no such restriction for shard keys. Therefore, I suggest retaining only the sort key check in the if-statement. If you encounter any issues with shard keys, could you please share the version of the adapter you're using and a model that triggers the problem?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants