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

Improve aggregation bind params #45

Merged
merged 4 commits into from
Feb 2, 2017

Conversation

smgallo
Copy link
Contributor

@smgallo smgallo commented Feb 1, 2017

Description

Not all information returned for an aggregation period was available as a bind parameter for aggregators. Now, any columns returned by getDirtyAggregationPeriods() will automatically be made available as a bind parameter for use in aggregation queries.

Motivation and Context

Laying the groundwork for using start_date and end_date in the new Accounts realm rather than start_date_ts and end_date_ts, which are not available.

Tests performed

Ran ingestion and aggregation of the past 100 days worth of jobs including experimental batch aggregation.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project as found in the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@smgallo smgallo added the enhancement Enhancement of the functionality of an existing feature label Feb 1, 2017
@smgallo smgallo added this to the v6.6.0 milestone Feb 1, 2017
// information will need to come from the last and first slice,
// respecitively (see note below).

$availableParamKeys = array_map(
Copy link
Contributor

Choose a reason for hiding this comment

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

This is done in two places, please make it a function.

// Make all of the data for each aggregation period available to the
// query. Change the array keys into bind parameters.

$availableParamKeys = array_map(
Copy link
Contributor

Choose a reason for hiding this comment

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

This is done in two places, please make it a function.

@smgallo smgallo merged commit 74f601f into ubccr:xdmod6.6 Feb 2, 2017
@smgallo smgallo deleted the etl/improve-aggregation-bind-params branch February 2, 2017 18:17
@smgallo smgallo mentioned this pull request Feb 2, 2017
6 tasks
plessbd pushed a commit to plessbd/xdmod that referenced this pull request Feb 3, 2017
* Make all aggregation period data available as bind parameters to query

* Fix phpcs issues

* Addressed code review comments (make redundant code into a function)
ryanrath pushed a commit to ryanrath/xdmod that referenced this pull request Apr 27, 2017
* Make all aggregation period data available as bind parameters to query

* Fix phpcs issues

* Addressed code review comments (make redundant code into a function)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement of the functionality of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants