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

Add version2 of function/aggregate metadata #1590

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

Conversation

martin-sucha
Copy link
Contributor

There were the following issues with handling of aggregates:

  • compileMetadata panicked if final_func was null.
    final_func is optional[1], so null is perfectly valid value.
  • The AggregateMetadata FinalFunc field could not store nil,
    so it is not possible to represent the null.
  • Functions and aggregates are not uniquely identified just by name.
    There could be two functions with the same name, but different
    argument types.

I'm adding a new version of function and aggregate metadata to fix
those shortcomings. For backward compatibility, I leave the V1
metadata filled-in where possible.

Fixes #1587

[1] https://cassandra.apache.org/doc/latest/cassandra/cql/functions.html#user-defined-aggregates-functions

There were the following issues with handling of aggregates:

- compileMetadata panicked if final_func was null.
  final_func is optional[1], so null is perfectly valid value.
- The AggregateMetadata FinalFunc field could not store nil,
  so it is not possible to represent the null.
- Functions and aggregates are not uniquely identified just by name.
  There could be two functions with the same name, but different
  argument types.

I'm adding a new version of function and aggregate metadata to fix
those shortcomings. For backward compatibility, I leave the V1
metadata filled-in where possible.

Fixes gocql#1587

[1] https://cassandra.apache.org/doc/latest/cassandra/cql/functions.html#user-defined-aggregates-functions
@Zariel
Copy link
Member

Zariel commented Apr 13, 2022

no we have semver can we just replace the broken api and bump the major version

@martin-sucha
Copy link
Contributor Author

That is true. We could bump the major version. If we do, we should also get rid of other deprecated APIs. Also, question is what gocql/v2 will mean in terms of the ecosystem, e.g. for updating gocqlx. cc @mmatczuk

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

Successfully merging this pull request may close these issues.

KeyspaceMetadata causes panic when describing aggregates
3 participants