[GitHub] [superset] villebro edited a comment on pull request #19324: feat: introduce hashids permalink keys

[GitBox <gi...@apache.org>]

villebro edited a comment on pull request #19324:
URL: https://github.com/apache/superset/pull/19324#issuecomment-1077797645


   > I'm wondering whether we can just generate a new salt and a new uuid for every permalink and save the hashid itself directly in the KeyValue table.
   
   Do you mean regenerate the salt each time? This would open up the theoretical risk of collisions, and would in practice be the same as using `token_urlsafe(11)`, thus removing the need for `hashids`, which is what @michael-s-molina originally proposed [here](https://github.com/apache/superset/pull/19078#discussion_r831038461).
   
   > We will add a new string column "key" in KeyValueEntry table with index=true, unique=true and nullable=false.
   
   I'm not opposed to introducing a new field in the table, but if it becomes very specific to permalinks, we may be better off with a dedicated table for them.
   
   > 1. We generate a uuid + a new salt
   
   Again, I may be missing something, but  since `hashids` only supports basing off of positive integers, we'd still need that serially generated id to generate the key, which in turn would require an update operation after creating a new entry. So if I understand what you're proposing correctly, for this use case it would be much simpler to just use `token_urlsafe(11)` and persist the UUID based on the token and namespace, after which it becomes directly queryable.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org