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

Default value of column should have additional information to identify if default value is auto-generated #933

Closed
pavish opened this issue Jan 3, 2022 · 4 comments · Fixed by #941
Assignees
Labels
type: bug Something isn't working work: backend Related to Python, Django, and simple SQL

Comments

@pavish
Copy link
Member

pavish commented Jan 3, 2022

Description

When the default value is auto-generated, the returned value is a string representing the procedure call.

Eg., for all PKs for the default id column, the value for default is "nextval('\"public\".data_types_id_seq'::regclass)".

We need additional information to help the client differentiate this from a normal string value.

Expected behavior

It would be better if default returned an object, something similar to:

default: {
   type: 'constant' // auto-generated or constant
   value: 'some default value'
}

For auto-generated values, we could send additional information based on how we intend to support formulas, procedures and functions in the future. For now, we can just send the same value and on the frontend the input can be disabled.

@pavish pavish added type: bug Something isn't working work: backend Related to Python, Django, and simple SQL status: draft labels Jan 3, 2022
@pavish pavish added this to the [07.1] 2022-01 improvements milestone Jan 3, 2022
@pavish
Copy link
Member Author

pavish commented Jan 3, 2022

@kgodey Marking this issue as draft inorder to decide on the format for the default value returned in the columns endpoint.

@mathemancer
Copy link
Contributor

mathemancer commented Jan 5, 2022

@pavish I have a quick implementation that uses:

...
"default": {
    "default_value": <some_val>
    "is_dynamic": <true|false>  // This is read-only
}
...

Does that work?

@mathemancer
Copy link
Contributor

As for the cruddy-looking output for dynamic defaults, I'm thinking we just use that for now so the user can at least see what's happening if they know how it works. We'll need to do other things when we have the ability to work with dynamic defaults better.

@pavish
Copy link
Member Author

pavish commented Jan 5, 2022

@mathemancer I think this format works. We could always extend this object when we need to provide additional information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working work: backend Related to Python, Django, and simple SQL
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants