Till Westmann has posted comments on this change.

Change subject: Add upsert option for feed
......................................................................


Patch Set 3:

(1 comment)

Looks good now. I've got one tiny stylistic proposal, but that's not important. 
However, it would be really nice if you could add a regression test that checks 
that the syntax work and that the corresponding feed indeed has upsert 
semantics.

I'm sorry about the trickle review - should have spotted the missing test at 
first glance ...

https://asterix-gerrit.ics.uci.edu/#/c/1068/3/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java
File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java:

Line 518:             }
As the difference between the branches is quite small, how about something like 
this:

    final Kind kind = isUpsertFeed ? Kind.UPSERT : Kind.INSERT;
    feedModificationOp = new InsertDeleteUpsertOperator(targetDatasource, 
        varRef, varRefsForLoading, metaExpSingletonList, kind, false);

with the corresponding import of the Kind enum?


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1068
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic5133e7c6941fea4110cc9983f99502f364dc810
Gerrit-PatchSet: 3
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Xikui Wang <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-Reviewer: abdullah alamoudi <[email protected]>
Gerrit-HasComments: Yes

Reply via email to