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

[BACKPORT 0.2.latest] Support for null value coalescing #161

Merged
merged 4 commits into from
Oct 2, 2023

Conversation

QMalcolm
Copy link
Collaborator

Resolves #142

Description

The lack of null value coalescing is a bug in 0.2.x because it makes some metrics incorrect. We're backporting the addition of these options to correct that. This is specifically a backport of #159

Checklist

@QMalcolm QMalcolm requested review from tlento and plypaul September 29, 2023 00:16
@cla-bot cla-bot bot added the cla:yes label Sep 29, 2023
Copy link
Collaborator

@tlento tlento left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable! I assume we won't be able to deploy this until core 1.6.x is updated, correct? Or is this also backwards compatible with existing installations? Like the parser should be fine until someone tries to actually set one of these parameters, but otherwise the defaults take over? When we add support for this to MetricFlow we'll need to ensure the MF dependency specifies the associated 0.2.x as the minimum version.

@QMalcolm
Copy link
Collaborator Author

QMalcolm commented Oct 2, 2023

Seems reasonable! I assume we won't be able to deploy this until core 1.6.x is updated, correct? Or is this also backwards compatible with existing installations? Like the parser should be fine until someone tries to actually set one of these parameters, but otherwise the defaults take over? When we add support for this to MetricFlow we'll need to ensure the MF dependency specifies the associated 0.2.x as the minimum version.

So from the core side, these options don't exist in 1.6 yet. That's okay because they are optional / will get defaulted. The parser will be fine unless someone tries to specify one of these. With that said the plan in core was to backport these options to 1.6. We've done the work to support them in the upcoming 1.7 release, and I was planning on backporting it to 1.6 later this week.

@tlento
Copy link
Collaborator

tlento commented Oct 2, 2023

Sounds good, merge when convenient!

@QMalcolm QMalcolm merged commit 31d4e9e into 0.2.latest Oct 2, 2023
10 checks passed
@QMalcolm QMalcolm deleted the 0.2.latest--backport-null-value-coalescing branch October 2, 2023 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants