Le mar. 17 déc. 2019 à 10:59, Vladimir Sitnikov <sitnikov.vladi...@gmail.com> a écrit : > > Michael>However, we know our > Michael>current contributors are reasonably fluent in Java. I'm not sure > about > Michael>Kotlin. > > 1) New contributions are not fluent in Quidem at all > 2) New contributions are not fluent in CalciteAssert at all
If you want to propose removing either of these, we could have a discussion about it, but you're talking about code which is already heavily used throughout Calcite. > 3) As I said, if someone does not want to get familiar with Kotlin, they > can write tests in Java (or XML or whatever we have) > 4) I'm sure even those who have never used Kotlin before could add tests to > by using battle-tested ctrl+c/v combinations: > https://github.com/vlsi/calcite/blob/0d6bec4140da46af07d58cf07a5e682d61529603/core/src/test/java/org/apache/calcite/test/SqlValidatorDynamicTest.kt#L55 > 5) If logic is required, then "if", "for", "try", "method calls" work very > similar to Java, and documentation is there on StackOverflow, Google, and > so on. I would agree with those points. > > Michael>I assume that similar concerns would have been raised > > Frankly speaking, I find those concerns too moot to ever rise. > Of course, it is valid that "adding a language" is a significant change, > however, I am not among those who would veto such changes based on very > generic statements like "ok, the new language would be hard for new > contributors" or "ok, the new language would complicate maintenance". > I assume everybody here knows "adding new languages" requires good > justification. > > I find it truly nonappealing to receive opinions like "it would complicate > maintenance" when not a single clue is given on how maintenance could screw > up. > If someone proposes COBOL, then I would definitely question that proposal :) > If someone proposes OCaml I would get curious how that would play rather > than declare "it is not proven yet to be the best option". > > At the same time, if someone is going to play "oh, X is a new language" > card, then I would highlight that Pig, Redis, Mongo, and Quidem are all > **languages** right inside Calcite's codebase. > It is not something theoretical, but I personally did run into exceptions > from Pig tests similar to https://issues.apache.org/jira/browse/CALCITE-3336 > Adding Kotlin to the mix of Pig, Piglet, and Mongo is innocent and it is a > small change :) > I didn't suggest I intend to veto any changes. A discussion was started, and I'm sharing my opinion. > > Recently the build system was switched from Maven to Gradle. > Before the migration there were lots of emails coming back and forth like > "oh, the release process is too complicated, it would take ages to fully > migrate to another build system" and so on. > > Have you seen the vote regarding Avatica release? It goes smooth. > Has Gradle resolved Maven's pain points I declared? It has. > Have you heard feedback on that change? The feedback seems to be positive > so far. Yes, and although I haven't had touch thee new build system much myself, it seems to have worked out very wlel. > > I assume we follow "commit then review" policy when committers commit > whatever they think is good, and in the rare cases when they are not sure > they discuss in JIRA, mailing list, and so on. Agreed. The consensus from the discussion you started seems to be that Kotlin should not be added to the tests. > > Michael>But I think that's a straw man. Rewriting some things in Kotlin > still > Michael>has some of the same disadvantages > > I'm sure we can't discuss "Rewriting some things" without mentioning the > "things" explicitly. > In my view, rewriting SqlValidatorMatchTest in Kotlin had no significant > enough disadvantages. > I agree, but you seem to be inferring that others think the "things" is "everything" when no one has said this. (It's certainly not the argument I'm making.) > Are there tests that degrade when rewritten? Of course, there are. > Should we discuss it like "has some of the same disadvantages"? I don't > think so. > > > Vladimir> > https://github.com/apache/calcite/pull/1657/commits/0d6bec4140da46af07d58cf07a5e682d61529603#diff-7a7027c499b6e4f5e7448b7b971052f1R85-R94 > Vladimir> much easier to read and update than similar tests in Java. > Michael>Assuming you understand, Kotlin, yes. > > Really? > Did you just say it requires one to understand Kotlin to successfully > copy-paste the following test? > > @Test > fun `ambiguous dynamic star4`() { > sql( > """ > select n.n_nation > from (select * from "SALES".NATION) as n, > (select * from "SALES".CUSTOMER) > """.trimIndent() > ).type("RecordType(ANY N_NATION) NOT NULL") > } No, I didn't say anything about copy and pasting. If all that is required for a test is copy and pasting, then sure this should be doable without knowing Kotlin. But many tests require more than this. > > I'm sure it does not require Kotlin understanding. > It is clear the test is doing something with SQL, and it has something to > do with the type of a record. > > Would Java make this test much more accessible? Of course, it would not. > > Java does not help to understand what the test is doing here. Java does not > help to understand what is really important in the test. > Java does not help to undestand which of the two stars is dynamic here, and > nothing explains why the test is "ambiguous". > > On the other hand, Kotlin improves test readability much, as the SQL is now > easy to get. I agree that for these specific tests, readability is improved although I don't find a huge difference myself. (Of course this is subjective.) > > Vladimir
