Skip to content

Conversation

@waralexrom
Copy link
Member

Check List

  • Tests have been run in packages where changes made if available
  • Linter has been run for changed code
  • Tests for the changes have been added if not covered yet
  • Docs have been added / updated if required

Issue Reference this PR resolves

[For example #12]

Description of Changes Made (if issue reference is not provided)

[Description goes here]

@waralexrom waralexrom requested review from a team as code owners February 19, 2025 17:12
@codecov
Copy link

codecov bot commented Feb 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.69%. Comparing base (08650e2) to head (b31a2bd).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #9244   +/-   ##
=======================================
  Coverage   83.69%   83.69%           
=======================================
  Files         228      228           
  Lines       82016    82016           
=======================================
  Hits        68640    68640           
  Misses      13376    13376           
Flag Coverage Δ
cubesql 83.69% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@waralexrom waralexrom force-pushed the tesseract-fallback-to-js-on-pre-aggregations branch from ab46752 to cc70404 Compare March 2, 2025 16:08
@waralexrom waralexrom force-pushed the tesseract-some-tests-fix branch from 3a99bef to e23eced Compare March 2, 2025 16:57
Copy link
Member

@KSDaemon KSDaemon left a comment

Choose a reason for hiding this comment

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

👍🏻 LGTM!

@@ -1,3 +1,4 @@
import { getEnv } from '@cubejs-backend/shared';
Copy link
Member

Choose a reason for hiding this comment

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

Did you forget to delete?

Ok(result)
}

fn deduplicate_deps_and_make_childs_tree(
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fn deduplicate_deps_and_make_childs_tree(
fn deduplicate_deps_and_make_children_tree(

&self,
call_deps: &Vec<CallDep>,
) -> Result<Vec<Vec<usize>>, CubeError> {
let mut childs_tree = Vec::new();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let mut childs_tree = Vec::new();
let mut children_tree = Vec::new();

let mut deduplicate_index_map = HashMap::<usize, usize>::new();
let mut deduplicate_map = HashMap::<CallDep, usize>::new();
for (i, dep) in call_deps.iter().enumerate() {
//If subcube used twice in function, then call_deps can hold duplicated dependencies
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
//If subcube used twice in function, then call_deps can hold duplicated dependencies
//If subcube is used twice in function, then call_deps can hold duplicated dependencies

@waralexrom waralexrom force-pushed the tesseract-fallback-to-js-on-pre-aggregations branch from cc70404 to 10bdac5 Compare March 3, 2025 13:00
Base automatically changed from tesseract-fallback-to-js-on-pre-aggregations to master March 3, 2025 13:48
@waralexrom waralexrom force-pushed the tesseract-some-tests-fix branch from e23eced to 403f3c4 Compare March 3, 2025 14:10
@waralexrom waralexrom merged commit 7248b7d into master Mar 3, 2025
74 checks passed
@waralexrom waralexrom deleted the tesseract-some-tests-fix branch March 3, 2025 15:44
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.

3 participants