-
Notifications
You must be signed in to change notification settings - Fork 13
matching stats as join table #3835
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
base: develop
Are you sure you want to change the base?
Conversation
# Conflicts: # backend/src/main/java/com/bakdata/conquery/mode/local/LocalStorageListener.java
… SqlMatchingStats easier
Also fixes a case, where missing entries were not registered to the root.
…tats-as-sql-function # Conflicts: # backend/src/main/java/com/bakdata/conquery/models/messages/namespaces/specific/UpdateMatchingStatsMessage.java
(experimental)
(this is a bit absurd tbh)
| @Data | ||
| public abstract class StorageListener<T extends Namespace>{ |
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.
Ich hätte das Interface beibehalten und davon diese abstract Class abgeleitet.
Interfaces können auch praktisch sein, da man für diese Proxies implementieren kann
| */ | ||
| //TODO better name | ||
| record Expression(ConceptElement<?> conceptElement, Map<Field<?>, Set<Param<?>>> conditions) { | ||
| public Expression and(Expression other) { |
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.
Braust du hier nicht eine parallel Struktur zu dem eigentlich Klassen Konstruct auf (AndCondition)?
| public Expression buildExpression(CTConditionContext context, ConceptElement<?> id) { | ||
| throw new IllegalStateException("Not implemented"); | ||
| } |
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.
buildExpression aufzurufen wirkt wie ein Minenfeld 👀
Ich bin aber auch verwirrt, weil ich nur einen Aufruf dafür finde, der wiederum in einem anderen buildExpression ist
| @JsonIgnore | ||
| private long numberOfEntities = -1L; | ||
|
|
||
| public synchronized long countEvents() { |
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.
müssen die synchronized sein?
| if (foundEntities.add(entityForEvent)) { | ||
| numberOfEntities++; | ||
| } | ||
| public void addEventFromBucket(String entityForEvent, Bucket bucket, int event, Iterable<Column> dateColumns) { |
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.
Bucket ist bei SQL aber kein Ding mehr oder? Kann die Logik an einen passenderen Ort gepackt werden?
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.
Löschen?
| .buildExpression(context, current) | ||
| .and(parentExpression); |
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.
Bei Equal wirst du hier aber viele Doppelungen haben, da die Parentnodes ein Superset der ChildNodes sind
| out.add(forCurrent); | ||
|
|
||
| for (ConceptTreeChild child : current.getChildren()) { | ||
| out.addAll(collectAllExpressions(child, forCurrent, context)); |
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.
Du kannst dir hier viele kurzlebige Allokationen sparen in dem du die Liste zum hinzufügen als Argument mitgibst und nicht in jeder Recursion eine weitere Liste erzeugst.
| return """ | ||
| CREATE OR REPLACE FUNCTION %s(%s) RETURNS output NVARCHAR(500) AS | ||
| BEGIN | ||
| output = %s; |
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.
Muss %s nicht sanitized werden?
| case REAL -> field.getDataType().isNumeric(); | ||
| case DECIMAL -> field.getDataType().isDecimal(); | ||
| case MONEY -> field.getDataType().isDecimal(); | ||
| case MONEY -> true; // TODO Need to find proper name |
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.
Ist das ein Workaround?
Eine Beispielhafte join-table könnte folgende Einträge enthalten:
im Falle von ColumnEquals wären dann noch weitere Spalten dabei. Bei PRESENT/EMPTY wären stattdessen true/false drin, die dann im join abgebildet werden.