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.
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.
Braust du hier nicht eine parallel Struktur zu dem eigentlich Klassen Konstruct auf (AndCondition)?
There was a problem hiding this comment.
nein, weil das nicht die Condition Logik abbildet. OR kann ich soweit ich das sehe nicht mit den join-tables abbilden und wird auch einfach nirgends verwendet.
There was a problem hiding this comment.
ebenso hab ich NOT nicht implmenetiert. Also es wird bei and bleiben, weil es die Logik vom Baum abbildet
| public Expression buildExpression(CTConditionContext context, ConceptElement<?> id) { | ||
| throw new IllegalStateException("Not implemented"); | ||
| } |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
na ich hab einfach noch nicht alles implementiert und wir werden auch nicht alles implementieren können. NOT wurde eh nur mit IS_EMPTY/_PRESENT in Verbindung verwendet. deswegen hab ich das unrolled. NotCondition kommt dann weg
| @JsonIgnore | ||
| private long numberOfEntities = -1L; | ||
|
|
||
| public synchronized long countEvents() { |
There was a problem hiding this comment.
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.
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.
Danke habs ordentlich aufgeräumt
There was a problem hiding this comment.
ne eigentlich eher die Logik wenn es dann passt hierhin migrieren
| .buildExpression(context, current) | ||
| .and(parentExpression); |
There was a problem hiding this comment.
Bei Equal wirst du hier aber viele Doppelungen haben, da die Parentnodes ein Superset der ChildNodes sind
There was a problem hiding this comment.
Die werden schon geschnitten in der Expression and Implementierung
| out.add(forCurrent); | ||
|
|
||
| for (ConceptTreeChild child : current.getChildren()) { | ||
| out.addAll(collectAllExpressions(child, forCurrent, context)); |
There was a problem hiding this comment.
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.
Muss %s nicht sanitized werden?
There was a problem hiding this comment.
die methode wird eh nicht mehr gebraucht aber für das was ich gemacht hatte nein, weil ich da jooq SQL reingesteckt hatte und das auch ausgeführt haben wollte
| 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.
Jooq hat kein money type und es gibt keine introspection, die mir money herausgibt
| private CDateRange span; | ||
|
|
||
| @JsonIgnore | ||
| private long numberOfEvents = -1L; |
There was a problem hiding this comment.
Vielleicht könnte man hier generell auf LongAdder umsteigen und auf die Map verzichten, aber ist nur so ein Gedanke. Würde glaube ich die Synchronisierungs- und Summen-Logik vereinfachen.
There was a problem hiding this comment.
Das ist moment leider nicht so easy, weil wir in der Logik ausnutzen, dass sich die Werte zur Laufzeit verändern können. Wenn ich da einen LongAdder verwenden will, muss ich davon ausgehen, dass es immer hochzählt.
Beim legacy Backend kommen ja von mehreren Servern nachrichten rein, die ich am besten genau einmal auswerten möchte.
| @Override | ||
| public WhereCondition convertToSqlCondition(CTConditionContext context) { | ||
| Field<String> field = DSL.field(DSL.name(context.getConnectorTable().getName(), column), String.class); | ||
| Field<String> field = (Field<String>) (Field<?>) field(DSL.name(column)); |
There was a problem hiding this comment.
| Field<String> field = (Field<String>) (Field<?>) field(DSL.name(column)); | |
| Field<String> field = field(DSL.name(column), String.class); |
There was a problem hiding this comment.
danke lol das habe ich komplett overengineered
...main/java/com/bakdata/conquery/models/datasets/concepts/conditions/ColumnEqualCondition.java
Outdated
Show resolved
Hide resolved
|
|
||
| @Override | ||
| public WhereCondition convertToSqlCondition(CTConditionContext context) { | ||
| Condition condition = DSL.field(DSL.name(column)).isNull(); |
There was a problem hiding this comment.
Hier hast du überall noch nicht den static import für DSL. gemacht, an anderer Stelle schon.
81dd8ff to
4366d85
Compare
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.