Channels
m
Mason Wheeler
05/20/2021, 7:39 PMI just found that
JdbcBufferedConsumerFactory.toWriteConfigNamingConventionTransformerNamingConventionTransformeru
user
05/20/2021, 8:29 PMlooking now
u
user
05/20/2021, 8:29 PMline 109 is this
u
user
05/20/2021, 8:29 PM Copy code
final String tableName = Names.concatQuotedNames("_airbyte_raw_", namingResolver.getIdentifier(streamName));u
user
05/20/2021, 8:30 PMwhich looks like it is calling the naming transformer, no?
u
user
05/20/2021, 8:30 PMThere are methods on the name transformer specifically for creating database table names. It's not using them; it's calling
getIdentifieru
user
05/20/2021, 8:31 PMAnd the things it's doing turn out to be incompatible with Oracle.
u
user
05/20/2021, 8:31 PMgot it.
u
user
05/20/2021, 8:31 PMso i think the table name ones are deprecated.
u
user
05/20/2021, 8:32 PMWell, can we un-deprecate them?
u
user
05/20/2021, 8:32 PMwhat's your goal here?
u
user
05/20/2021, 8:32 PMare they producing valid names and getidentifier isn't?
u
user
05/20/2021, 8:32 PMlooking at the implementation of those methods it looks like they do pretty much the same thing as
getIdentifieru
user
05/20/2021, 8:32 PM...unless you override them
u
user
05/20/2021, 8:33 PMLook at all the stuff it's doing after calling
getIdentifieru
user
05/20/2021, 8:33 PM1. In Oracle, it's not valid to start a table name with an underscore.
2. Table names have a strict 30 character length limit.
u
user
05/20/2021, 8:33 PMThe current system is breaking both of those. (And possibly other things once I get past the first two.)
u
user
05/20/2021, 8:33 PMah.
u
user
05/20/2021, 8:34 PMokay. so issue is that on line 109 we are prepending the table name with something that's not valid for oracle.
u
user
05/20/2021, 8:34 PMSo for the Oracle destination, I need very precise control over table name generation
u
user
05/20/2021, 8:38 PMThat's the first issue. The second issue is that temp tables end up with a ginormous name like
airbyte_1234567890_airbyte_table_nameu
user
05/20/2021, 8:39 PMyeah... 30 characters it not a lot of characters.
u
user
05/20/2021, 8:40 PMu
user
05/20/2021, 8:41 PMis this article accurate? ^
u
user
05/20/2021, 8:41 PMwould it be sane for us to just support oracle 12.2 and above?
u
user
05/20/2021, 8:42 PMWell that's interesting
u
user
05/20/2021, 8:42 PMi guess whatever container you're using is maxing out at 30 characters?
u
user
05/20/2021, 8:44 PMHmm... looks like the Docker container I found is version 11. I'll see if there's a more recent one. It looks like that answer was updated in 2017 with the higher limit, so it could be that this is (relatively) new.
u
user
05/20/2021, 8:50 PMit looks like davin and subodh just slipped this hack in for handling name length
u
user
05/20/2021, 8:51 PMah.
u
user
05/20/2021, 8:51 PMactually that's pretty bad.
u
user
05/20/2021, 8:51 PMweshouldn't keep doing this.
u
user
05/20/2021, 8:52 PMyeah, I agree
u
user
05/20/2021, 8:53 PMi'm wondering if we can do something like this https://github.com/airbytehq/airbyte/pull/3515
u
user
05/20/2021, 8:54 PMI think the best thing to do would be just to move this logic back to the table name generation functions and un-deprecate them. Looking at the deprecation message, it basically says "we don't like this because it's very SQL-specific and not everything we connect to is a SQL database." But for stuff that is a SQL database, there are times you legitimately need SQL-specific stuff. And if you're not a SQL database, just don't use that part.
u
user
05/20/2021, 8:55 PMdoes my suggestion not achieve the same thing?
u
user
05/20/2021, 8:56 PMyou mean that PR?
u
user
05/20/2021, 8:56 PMyeah
u
user
05/20/2021, 8:56 PMyou know what i think you're right, it's fine to just use those deprecated methods for now i think that's okay.
u
user
05/20/2021, 8:56 PMwe can spend some time cleaning this up after.
u
user
05/20/2021, 8:57 PM...kind of, but now you need a bunch of special-case logic in
getIdentifieru
user
05/20/2021, 8:57 PMwhat's the specific prefix?
u
user
05/20/2021, 8:58 PMisn't it just that oracle doesn't let you use special characters in names?
u
user
05/20/2021, 8:58 PMYou can use special characters; you just can't start with an underscore.
u
user
05/20/2021, 8:58 PMha
u
user
05/20/2021, 8:59 PMAnd I'm still thinking about the length restriction, just in case we're stuck with pre 12.2 versions
u
user
05/20/2021, 8:59 PMok
u
user
05/20/2021, 9:01 PMChecking with a different container name now, to see if I can get this working
u
user
05/20/2021, 9:34 PMIf I change the name of the test container, and suddenly it seems to hang (spinning for 10+ minutes at the "Instantiating tests..." phase) when I run tests, what's the most likely problem?
u
user
05/20/2021, 9:42 PMhow are you changing the name?
u
user
05/20/2021, 9:43 PMdb = new OracleContainer("image name goes here");u
user
05/20/2021, 9:44 PMIt finally got past that and gave an error:
java.sql.SQLException: Cannot create PoolableConnectionFactory (Listener refused the connection with the following error:
ORA-12514, TNS:listener does not currently know of service requested in connect descriptor
)
u
user
05/20/2021, 9:46 PMso it didn't hang forever, it eventually did start?
u
user
05/20/2021, 9:46 PMyeah
u
user
05/20/2021, 9:48 PMgot it. is the image you are passing in there supposed to work?
u
user
05/20/2021, 9:48 PMwith the original image you are were able to log in just fine
u
user
05/20/2021, 9:48 PMnew image doesn't?
u
user
05/20/2021, 9:48 PMso i maybe there's some reason that image incompatible with test containers?
u
user
05/20/2021, 9:48 PMYeah. I'm looking around on DockerHub for Oracle 12 images. Haven't found one yet that works.
u
user
05/20/2021, 9:49 PMBut there are a bunch so I'll keep looking...
u
user
05/20/2021, 9:49 PMkk.
3 Views