Autodiscover tables for segments w/ complex segment expressions #13664
Conversation
…an be created in Archivers.
diosmosis
changed the title
|
Can you add some tests for this @diosmosis ? |
|
👍 will do |
|
Added tests. |
core/Segment/SegmentExpression.php
Outdated
| */ | ||
| private function parseColumnsFromSqlExpr($field) | ||
| { | ||
| preg_match_all('/\b([a-zA-Z0-9_`]+\.[a-zA-Z0-9_`]+)\b/', $field, $matches); |
There was a problem hiding this comment.
@diosmosis not sure if on purpose... when field is eg "log_action.type" then
preg_match_all('/\b([a-zA-Z0-9_]+.[a-zA-Z0-9_]+)\b/', 'log_action.type', $matches);
returns this:
also see https://3v4l.org/0v32G
not sure if the field can be like this.
There was a problem hiding this comment.
Updated to better handle backticks (it's not super intelligent, but should work given this is for plugin devs).
|
Tested and worked. Only thing I would have done be to maybe write some tests for I used xdebug to see what the inputs are, how they are generated etc. to see about possible inputs etc... seeing maybe just a few examples in tests might help understand what is being down there but all good if not adding more tests and if it is indirectly covered by other tests. |
All it should do is look for |
|
oki 👍 |
|
Added a test as you suggested. Will merge if the build passes. |
Currently if a segment has a custom
$sqlSegmentthat includes functions/operations (likeUNIX_TIMESTAMP(log_visit.visit_first_action_time) - log_visit.days_since_first_visit * 86400), the joins for segment expressions won't be discovered. So if we try to use this segment w/ log_conversion, it will skip the log_visit join and fail.This PR fixes that by parsing out table/column names from the segment expression.
Also includes a new public method to return the ArchiveWriter instance in ArchiveProcessor so it's easier to create new ArchiveProcessor instances from within Archivers.