Nifi 2673: Add MERGE support on processor ConvertJSONTOSQL

[paulgibeault]

These changes provide ASNI MERGE support to the processor ConvertJSONToSQL. This has been tested against Teradata Version 15.10.02.06 only.

[paulgibeault]

Is there a chance of getting this pull request included in the 1.1.0 release of NiFi?

[brosander]

Reviewing

[brosander]

Hey, @paulgibeault,

Thanks for the contribution! I think this is valuable functionality but I'm currently having some issues with it.

I'm currently testing the merge against Oracle express 11g and am having some problems.

It's generating the following SQL that Oracle doesn't like:

MERGE VIOLATIONS target_t 
USING VALUES (?,?) 
AS source_t (SUMMONS_NUMBER,PLATE) 
ON target_t.SUMMONS_NUMBER = source_t.SUMMONS_NUMBER 
WHEN MATCHED THEN 
UPDATE SET PLATE = source_t.PLATE 
WHEN NOT MATCHED THEN 
INSERT (SUMMONS_NUMBER,PLATE) 
VALUES (source_t.SUMMONS_NUMBER,source_t.PLATE) 
;

Oracle is complaining that it is missing an INTO keyword after the MERGE.

If I add that manually, it then complains of an invalid table name. I suspect this is due to the "USING VALUES" portion of the query which doesn't appear to be standard.

If each database is going to require different SQL for this operation, it may warrant its own processor where we can contain all of that custom logic per database, possibly with the DatabaseAdapter interface (@mattyb149 may have some thoughts around that).

If you'd like to repro my tests, here is a link to the Oracle docker image and here is one to my flow.xml.gz and test.json file.

Thanks,
Bryan

[mattyb149]

It's a real bummer that an ANSI "standard" such as MERGE is done differently by so many databases :( If there isn't an application of the standard MERGE syntax that applies to many databases (or the functionality is desired for DBs that don't support the standard), then I like Bryan's suggestion of using the DatabaseAdapter interface. We could add a method such as getMergeStatement() to DatabaseAdapter, with no-op (or exception-throwing) default implementation (on the interface or the subclasses), then implement for Oracle (which has a DatabaseAdapter already) and add a TeradataDatabaseAdapter subclass to implement their version of the syntax. Thoughts?

Also, it sounds like the community is gearing up for a 1.1.0 release soon, do you think you'd have any updates ready in the near-term? If not (which is totally fine btw), could we push the Fix Version for the Jira to 1.2.0?

[patricker]

Paul is on vacation, but I worked with him on this.

After looking over the syntax I can see why this worked for us but not for others. It looks like we used a syntax for Teradata that allows you to MERGE single rows, USING VALUES, that isn't done the same way on other platforms. We derived it from the standard MERGE syntax, but didn't notice when we deviated from the standard.

For reference: the syntax for MS SQL Server seems to be http://stackoverflow.com/a/5981300/328968.

I don't see any issues with moving this into a DatabaseAdapter, but it would also make sense to me to have a default, maybe something closer to the MS SQL one(?) that could be used if no DatabaseAdapter is found. I'm not sure where it makes sense for that default implementation to live.

[brosander]

If the standard syntax works for MS SQL Server, we could even make that the default implementation on the interface itself. That way any ANSI-compliant database types would "just work".