optimize sort #55
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,7 +14,7 @@ node_js: | |
| - '4.2' | ||
|
|
||
| before_script: | ||
| - npm install juttle@^0.7.0 | ||
|
|
||
| script: | ||
| - npm run test-coverage | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,14 +48,12 @@ class ReadSql extends AdapterRead { | |
| this.addOptimizations(params.optimization_info); | ||
|
|
||
| this.baseQuery = this.baseQuery.from(options.table); | ||
|
|
||
| this.addSorting(); | ||
| } | ||
|
|
||
| periodicLiveRead() { | ||
| return !!this.timeField && !this.sortFields; | ||
| } | ||
|
|
||
| getDbConnection() { | ||
|
|
@@ -95,6 +93,20 @@ class ReadSql extends AdapterRead { | |
| this.timeField = options.timeField || | ||
| (options.from || options.to || options.last ? 'time' : undefined); | ||
| } | ||
|
|
||
| addSorting() { | ||
| if (this.sortFields) { | ||
| // adds multiple sort orders | ||
| this.sortFields.forEach((sortObj) => { | ||
|
There was a problem hiding this comment. I would put this down by line 128 in There was a problem hiding this comment. Oh. I thought having all the order by statements together would be easiest to read since this function determines sort order. No? There was a problem hiding this comment. You're the boss! In my head even though they both use the |
||
| this.baseQuery = this.baseQuery.orderBy(sortObj.field, sortObj.direction); | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| if (this.timeField && this.optimization_info.type !== 'reduce') { | ||
| this.baseQuery = this.baseQuery.orderBy(this.timeField, this.tailOptimized ? 'desc' : 'asc'); | ||
| } | ||
| } | ||
|
|
||
| addFilters(filter_ast) { | ||
| let compiler = new FilterSQLCompiler({ baseQuery: this.baseQuery }); | ||
|
|
@@ -110,6 +122,15 @@ class ReadSql extends AdapterRead { | |
| this.logger.debug(optimization_info.type + ' optimization, new max size: ', optimization_info.limit); | ||
| this.maxSize = optimization_info.limit; | ||
| this.tailOptimized = optimization_info.type === 'tail'; | ||
| return; | ||
| } | ||
| if (optimization_info.type === 'sort') { | ||
| this.logger.debug(optimization_info.type + ' optimization, params: ', optimization_info); | ||
| this.sortFields = optimization_info.columns; | ||
| if (optimization_info.limit) { | ||
| this.maxSize = optimization_info.limit; | ||
| } | ||
| return; | ||
| } | ||
| if (optimization_info.type === 'reduce') { | ||
| let groupby = optimization_info.groupby; | ||
|
|
@@ -335,6 +356,13 @@ class ReadSql extends AdapterRead { | |
| readEnd: this.timeField ? to : new JuttleMoment(Infinity) | ||
| }; | ||
| } | ||
|
|
||
| if (this.sortFields) { | ||
|
There was a problem hiding this comment. Can SQL not paginate sorted data? There was a problem hiding this comment. You mean using offset? Yes but using offset when data could be coming in in real time could cause errors in the pagination. this method takes real time data into account so IMO it's a superior pagination method. There was a problem hiding this comment. I guess I don't understand what this is doing...if we read with a limit of There was a problem hiding this comment. @davidvgalbraith you're absolutely right - the pagination was broken and untested. Fixed now. Take another look? |
||
| let res = this.processPaginatedResults(points, this.sortFields[0].field); | ||
| res.readEnd = to; | ||
| return res; | ||
| } | ||
|
|
||
| //perform time-based pagination if timeField is indicated | ||
| if (this.timeField) { | ||
| let res = this.processPaginatedResults(points, this.timeField); | ||
|
|
@@ -404,6 +432,12 @@ class ReadSql extends AdapterRead { | |
| ) { | ||
| points = this.parseTime(points, { timeField: this.timeField }); | ||
| } | ||
|
|
||
| if (this.sortFields) { | ||
| _.each(points, (p) => { | ||
| delete p.time; | ||
| }); | ||
| } | ||
|
|
||
| this.total_emitted_points += points.length; | ||
| return points; | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a little more canonical to say
if (graph.node_has_option(sort, 'groupby')) {in cases like this.