Re: CEP-15 multi key transaction syntax
Love the Oracle/Postgres RETURNING syntax and just generally +1 to adding INSERT and DELETE. And for each DML type... > - INSERT ... RETURNING returns inserted data (useful for defaulted or > autoincrement columns). > - UPDATE ... RETURNING returns the modified data. > - DELETE ... RETURNING returns the now-deleted data. > >
Re: CEP-15 multi key transaction syntax
All of my text below largely extends your question of syntax in a few directions: - What is the user experience of trying to run different statements with this syntax? - How do transactions interact with other Cassandra constructs? - What are the execution semantics of these statements? which I do acknowledge is a moderate re-scoping of the question. Also, please take my understanding of existing CQL and DDL constructs with an impractically large grain of salt. Undesireable Transactions - I tried to match CQL docs up against a number of ways of writing statements which Accord wouldn't like, or users might not like the effect of running. I'm assuming it'd be good to think through how one would express the error message or guidance given to users? Or at least just making sure I understand correctly what is writable but not executable or desirable. === Likely Unexecutable All the cases here are predicated on the lack of automatic reconnaissance transaction support. 1. Dependant SELECTs CREATE TABLE users (name text primary key, home_state text); CREATE TABLE states (name text primary key, population int); BEGIN TRANSACTION; /*1*/ SELECT home_state FROM users WHERE name='blake' AS user; /*2*/ SELECT population FROM states WHERE name=user.home_state AS state; COMMIT TRANSACTION; The primary key for SELECT (2) depends on a value produced by SELECT (1), which results in not being able to produce the full read conflict set ahead of time for Accord. 2. Dependant UPDATEs CREATE TABLE users (name text primary key, home_state text); CREATE TABLE states (name text primary key, population int); BEGIN TRANSACTION; SELECT home_state FROM users WHERE name='blake' AS user; UPDATE states SET population += 1 WHERE name=user.home_state AS state; COMMIT TRANSACTION; The primary key for UPDATE depends on a value produced by SELECT, which results in not being able to produce the full write conflict set ahead of time for Accord. 3. UPDATE from secondary index (or SASI) CREATE TABLE users (id int primary key, name text, home_state text); CREATE INDEX users_by_name ON users (name); BEGIN TRANSACTION; UPDATE users SET miles_driven += 30 WHERE name='blake'; COMMIT TRANSACTION; This is just a rephasing of (2), but hiding the SELECT behind an implict query to the secondary index for the primary key. (But an UPDATE from a covering index would be okay!) 4. The presence of a materialized view which can implicitly add any/all of the above CREATE TABLE users ( name text primary key, miles_driven int, state text, ); CREATE MATERIALIZED VIEW users_by_home_state_by_miles AS SELECT * FROM users WHERE home_state IS NOT NULL PRIMARY KEY (home_state, miles_driven, name); BEGIN TRANSACTION; UPDATE users SET miles_driven += 30 WHERE name='blake'; COMMIT TRANSACTION; This is a rephrasing of (2), but the UPDATE is to the materialized view, and the SELECT is to get miles_driven out of the UPDATE on users. Some materialized views would be fine to transactionally update though. === Poor Performance 5. UPDATE with predicate on non-primary key CREATE TABLE users ( id int primary key, name text, miles_driven int ); BEGIN TRANSACTION; UPDATE users SET miles_driven += 30 WHERE name='blake'; COMMIT TRANSACTION; As now any transaction which touches the 'blake' row in `users` is going to have to wait behind a full table scan completing in Accord's transaction executor. Then any transaction which conflicts with one of those would also have to wait, and eventually snowball into a cascading stall in transaction processing. Support for these kinds of things could be useful to some users though? It might be wise to consider extending Accord with a table-level lock concept to avoid having to maintain conflict information on every key in the table individually. 6. Large SELECTs Are Actually Okay But Look Like They Shouldn't Be CREATE TABLE users (name text primary key, state text, miles_driven int); BEGIN TRANSACTION; SELECT sum(miles_driven) FROM users WHERE state='Ohio'; COMMIT TRANSACTION; As read-only transactions should be cheap, and I don't think the computation would be much more notably expensive than the non-transactional version of this. However, what maybe feels similar to a user: BEGIN TRANSACTION; SELECT sum(miles_driven) FROM users WHERE state='Ohio'; UPDATE miles_by_state SET total=sum(miles_driven) WHERE state='Ohio'; COMMIT TRANSACTION; does mean we're back in the bad idea category. === I Have Literally No Idea 7. Triggers What are the transactional guarantees of triggers? These are implemented in Java, so that'd just be outright banned by Accord? Unless triggers can have an API to spit out extra conflict ranges for the partition they live on? Sounds like an Accord
Re: CEP-15 multi key transaction syntax
Oops. I missed this part: "full primary key or a limit of 1" Still curious what the end-user would see if there is more than one row returned. On Sat, Jun 4, 2022 at 5:46 PM Patrick McFadin wrote: > I've been waiting for this email! I'll echo what Jeff said about how > exciting this is for the project. > > On the SELECT inside the transaction: > > In the first example, I'm making an assumption that you are doing a select > on a partition key and only expect one result but is any valid CQL SELECT > allowed here? If 'model' were a non-partition key column name and was > indexed, then you could potentially have multiple rows returned and that > isn't an allowed operation. Are only partition key lookups allowed or is > there some logic looking for only one row? > > I'm asking because I can see in reverse time series models where you can > select the latest temperature > SELECT temperature FROM weather_station WHERE id=1234 AND > DATE='2022-06-04' LIMIT 1; > > (also, horrible example. Everyone knows that the return value for a > Pinto.is_running will always evaluate to FALSE) > > On COMMIT TRANSACTION: > > So much to unpack here. In the case that the condition is met, is the > mutation applied at that point, or has it already happened and there is > something like a rollback segment? What is the case when the condition is > not met and what is presented to the end-user? More importantly, what > happens with respect to the A & I in ACID when the transaction is applied? > > If UPDATE is used, returning the number of rows changed would be helpful. > > Is this something that can be done interactively in cqlsh or does it all > have to be submitted in one statement block? > > I'll stop here for now. > > Patrick > > On Sat, Jun 4, 2022 at 3:34 PM bened...@apache.org > wrote: > >> > The returned result set is after the updates are applied? >> >> Returning the prior values is probably more powerful, as you can perform >> unconditional updates and respond to the prior state, that you otherwise >> would not know. It’s also simpler to implement. >> >> >> >> My inclination is to require that SELECT statements are declared first, >> so that we leave open the option of (in future) supporting SELECT >> statements in any place in the transaction, returning the values as of >> their position in a sequential execution of the statements. >> >> >> >> > And would you allow a transaction that had > 1 named select and no >> modification statements, but commit if 1=1 ? >> >> >> >> My preference is that the IF condition is anyway optional, as it is much >> more obvious to a user than concocting some always-true condition. But yes, >> read-only transactions involving multiple tables will definitely be >> supported. >> >> >> >> >> >> *From: *Jeff Jirsa >> *Date: *Saturday, 4 June 2022 at 22:49 >> *To: *dev@cassandra.apache.org >> *Subject: *Re: CEP-15 multi key transaction syntax >> >> >> And would you allow a transaction that had > 1 named select and no >> modification statements, but commit if 1=1 ? >> >> > On Jun 4, 2022, at 2:45 PM, Jeff Jirsa wrote: >> > >> > >> > >> >> On Jun 3, 2022, at 8:39 AM, Blake Eggleston >> wrote: >> >> >> >> Hi dev@, >> > >> > First, I’m ridiculously excited to see this. >> > >> >> >> >> I’ve been working on a draft syntax for Accord transactions and wanted >> to bring what I have to the dev list to solicit feedback and build >> consensus before moving forward with it. The proposed transaction syntax is >> intended to be an extended batch syntax. Basically batches with selects, >> and an optional condition at the end. To facilitate conditions against an >> arbitrary number of select statements, you can also name the statements, >> and reference columns in the results. To cut down on the number of >> operations needed, select values can also be used in updates, including >> some math operations. Parameterization of literals is supported the same as >> other statements. >> >> >> >> Here's an example selecting a row from 2 tables, and issuing updates >> for each row if a condition is met: >> >> >> >> BEGIN TRANSACTION; >> >> SELECT * FROM users WHERE name='blake' AS user; >> >> SELECT * from cars WHERE model='pinto' AS car; >> >> UPDATE users SET miles_driven = user.miles_driven + 30 WHERE >> name='blake'; >> >> UPDATE cars SET miles_driven = car.miles_driven + 30 WHERE >> model='pinto'; >> >> COMMIT TRANSACTION IF car.is_running; >> >> >> >> This can be simplified by naming the updates with an AS syntax. >> If updates are named, a corresponding read is generated behind the scenes >> and its values inform the update. >> >> >> >> Here's an example, the query is functionally identical to the previous >> query. In the case of the user update, a read is still performed behind the >> scenes to enable the calculation of miles_driven + 30, but doesn't need to >> be named since it's not referenced anywhere else. >> >> >> >> BEGIN TRANSACTION; >> >> UPDATE users SET miles_driven += 30 WHERE
Re: CEP-15 multi key transaction syntax
I've been waiting for this email! I'll echo what Jeff said about how exciting this is for the project. On the SELECT inside the transaction: In the first example, I'm making an assumption that you are doing a select on a partition key and only expect one result but is any valid CQL SELECT allowed here? If 'model' were a non-partition key column name and was indexed, then you could potentially have multiple rows returned and that isn't an allowed operation. Are only partition key lookups allowed or is there some logic looking for only one row? I'm asking because I can see in reverse time series models where you can select the latest temperature SELECT temperature FROM weather_station WHERE id=1234 AND DATE='2022-06-04' LIMIT 1; (also, horrible example. Everyone knows that the return value for a Pinto.is_running will always evaluate to FALSE) On COMMIT TRANSACTION: So much to unpack here. In the case that the condition is met, is the mutation applied at that point, or has it already happened and there is something like a rollback segment? What is the case when the condition is not met and what is presented to the end-user? More importantly, what happens with respect to the A & I in ACID when the transaction is applied? If UPDATE is used, returning the number of rows changed would be helpful. Is this something that can be done interactively in cqlsh or does it all have to be submitted in one statement block? I'll stop here for now. Patrick On Sat, Jun 4, 2022 at 3:34 PM bened...@apache.org wrote: > > The returned result set is after the updates are applied? > > Returning the prior values is probably more powerful, as you can perform > unconditional updates and respond to the prior state, that you otherwise > would not know. It’s also simpler to implement. > > > > My inclination is to require that SELECT statements are declared first, so > that we leave open the option of (in future) supporting SELECT statements > in any place in the transaction, returning the values as of their position > in a sequential execution of the statements. > > > > > And would you allow a transaction that had > 1 named select and no > modification statements, but commit if 1=1 ? > > > > My preference is that the IF condition is anyway optional, as it is much > more obvious to a user than concocting some always-true condition. But yes, > read-only transactions involving multiple tables will definitely be > supported. > > > > > > *From: *Jeff Jirsa > *Date: *Saturday, 4 June 2022 at 22:49 > *To: *dev@cassandra.apache.org > *Subject: *Re: CEP-15 multi key transaction syntax > > > And would you allow a transaction that had > 1 named select and no > modification statements, but commit if 1=1 ? > > > On Jun 4, 2022, at 2:45 PM, Jeff Jirsa wrote: > > > > > > > >> On Jun 3, 2022, at 8:39 AM, Blake Eggleston > wrote: > >> > >> Hi dev@, > > > > First, I’m ridiculously excited to see this. > > > >> > >> I’ve been working on a draft syntax for Accord transactions and wanted > to bring what I have to the dev list to solicit feedback and build > consensus before moving forward with it. The proposed transaction syntax is > intended to be an extended batch syntax. Basically batches with selects, > and an optional condition at the end. To facilitate conditions against an > arbitrary number of select statements, you can also name the statements, > and reference columns in the results. To cut down on the number of > operations needed, select values can also be used in updates, including > some math operations. Parameterization of literals is supported the same as > other statements. > >> > >> Here's an example selecting a row from 2 tables, and issuing updates > for each row if a condition is met: > >> > >> BEGIN TRANSACTION; > >> SELECT * FROM users WHERE name='blake' AS user; > >> SELECT * from cars WHERE model='pinto' AS car; > >> UPDATE users SET miles_driven = user.miles_driven + 30 WHERE > name='blake'; > >> UPDATE cars SET miles_driven = car.miles_driven + 30 WHERE > model='pinto'; > >> COMMIT TRANSACTION IF car.is_running; > >> > >> This can be simplified by naming the updates with an AS syntax. > If updates are named, a corresponding read is generated behind the scenes > and its values inform the update. > >> > >> Here's an example, the query is functionally identical to the previous > query. In the case of the user update, a read is still performed behind the > scenes to enable the calculation of miles_driven + 30, but doesn't need to > be named since it's not referenced anywhere else. > >> > >> BEGIN TRANSACTION; > >> UPDATE users SET miles_driven += 30 WHERE name='blake'; > >> UPDATE cars SET miles_driven += 30 WHERE model='pinto' AS car; > >> COMMIT TRANSACTION IF car.is_running; > >> > >> Here’s another example, performing the canonical bank transfer: > >> > >> BEGIN TRANSACTION; > >> UPDATE accounts SET balance += 100 WHERE name='blake' AS blake; > >> UPDATE accounts SET balance -= 100 WHERE name='benedict' AS
Re: CEP-15 multi key transaction syntax
> The returned result set is after the updates are applied? Returning the prior values is probably more powerful, as you can perform unconditional updates and respond to the prior state, that you otherwise would not know. It’s also simpler to implement. My inclination is to require that SELECT statements are declared first, so that we leave open the option of (in future) supporting SELECT statements in any place in the transaction, returning the values as of their position in a sequential execution of the statements. > And would you allow a transaction that had > 1 named select and no > modification statements, but commit if 1=1 ? My preference is that the IF condition is anyway optional, as it is much more obvious to a user than concocting some always-true condition. But yes, read-only transactions involving multiple tables will definitely be supported. From: Jeff Jirsa Date: Saturday, 4 June 2022 at 22:49 To: dev@cassandra.apache.org Subject: Re: CEP-15 multi key transaction syntax And would you allow a transaction that had > 1 named select and no modification statements, but commit if 1=1 ? > On Jun 4, 2022, at 2:45 PM, Jeff Jirsa wrote: > > > >> On Jun 3, 2022, at 8:39 AM, Blake Eggleston wrote: >> >> Hi dev@, > > First, I’m ridiculously excited to see this. > >> >> I’ve been working on a draft syntax for Accord transactions and wanted to >> bring what I have to the dev list to solicit feedback and build consensus >> before moving forward with it. The proposed transaction syntax is intended >> to be an extended batch syntax. Basically batches with selects, and an >> optional condition at the end. To facilitate conditions against an arbitrary >> number of select statements, you can also name the statements, and reference >> columns in the results. To cut down on the number of operations needed, >> select values can also be used in updates, including some math operations. >> Parameterization of literals is supported the same as other statements. >> >> Here's an example selecting a row from 2 tables, and issuing updates for >> each row if a condition is met: >> >> BEGIN TRANSACTION; >> SELECT * FROM users WHERE name='blake' AS user; >> SELECT * from cars WHERE model='pinto' AS car; >> UPDATE users SET miles_driven = user.miles_driven + 30 WHERE name='blake'; >> UPDATE cars SET miles_driven = car.miles_driven + 30 WHERE model='pinto'; >> COMMIT TRANSACTION IF car.is_running; >> >> This can be simplified by naming the updates with an AS syntax. If >> updates are named, a corresponding read is generated behind the scenes and >> its values inform the update. >> >> Here's an example, the query is functionally identical to the previous >> query. In the case of the user update, a read is still performed behind the >> scenes to enable the calculation of miles_driven + 30, but doesn't need to >> be named since it's not referenced anywhere else. >> >> BEGIN TRANSACTION; >> UPDATE users SET miles_driven += 30 WHERE name='blake'; >> UPDATE cars SET miles_driven += 30 WHERE model='pinto' AS car; >> COMMIT TRANSACTION IF car.is_running; >> >> Here’s another example, performing the canonical bank transfer: >> >> BEGIN TRANSACTION; >> UPDATE accounts SET balance += 100 WHERE name='blake' AS blake; >> UPDATE accounts SET balance -= 100 WHERE name='benedict' AS benedict; >> COMMIT TRANSACTION IF blake EXISTS AND benedict.balance >= 100; >> >> As you can see from the examples, column values can be referenced via a dot >> syntax, ie: . -> select1.value. Since the read portion >> of the transaction is performed before evaluating conditions or applying >> updates, values read can be freely applied to non-primary key values in >> updates. Select statements used either in checking a condition or creating >> an update must be restricted to a single row, either by specifying the full >> primary key or a limit of 1. Multi-row selects are allowed, but only for >> returning data to the client (see below). >> >> For evaluating conditions, = & != are available for all types, <, <=, >, >= >> are available for numerical types, and EXISTS, NOT EXISTS can be used for >> partitions, rows, and values. If any column references cannot be satisfied >> by the result of the reads, the condition implicitly fails. This prevents >> having to include a bunch of exists statements. > > Is there a new keyword for “partition (not) exists” or is it inferred by the > select? > >> >> On completion, an operation would return a boolean value indicating the >> operation had been applied, and a result set for each named select (but not >> named update). We could also support an optional RETURN keyword, which would >> allow the user to only return specific named selects (ie: RETURN select1, >> select2). >> > > The returned result set is after the updates are applied? > > >> Let me know what you think! >> >> Blake
Re: CEP-15 multi key transaction syntax
And would you allow a transaction that had > 1 named select and no modification statements, but commit if 1=1 ? > On Jun 4, 2022, at 2:45 PM, Jeff Jirsa wrote: > > > >> On Jun 3, 2022, at 8:39 AM, Blake Eggleston wrote: >> >> Hi dev@, > > First, I’m ridiculously excited to see this. > >> >> I’ve been working on a draft syntax for Accord transactions and wanted to >> bring what I have to the dev list to solicit feedback and build consensus >> before moving forward with it. The proposed transaction syntax is intended >> to be an extended batch syntax. Basically batches with selects, and an >> optional condition at the end. To facilitate conditions against an arbitrary >> number of select statements, you can also name the statements, and reference >> columns in the results. To cut down on the number of operations needed, >> select values can also be used in updates, including some math operations. >> Parameterization of literals is supported the same as other statements. >> >> Here's an example selecting a row from 2 tables, and issuing updates for >> each row if a condition is met: >> >> BEGIN TRANSACTION; >> SELECT * FROM users WHERE name='blake' AS user; >> SELECT * from cars WHERE model='pinto' AS car; >> UPDATE users SET miles_driven = user.miles_driven + 30 WHERE name='blake'; >> UPDATE cars SET miles_driven = car.miles_driven + 30 WHERE model='pinto'; >> COMMIT TRANSACTION IF car.is_running; >> >> This can be simplified by naming the updates with an AS syntax. If >> updates are named, a corresponding read is generated behind the scenes and >> its values inform the update. >> >> Here's an example, the query is functionally identical to the previous >> query. In the case of the user update, a read is still performed behind the >> scenes to enable the calculation of miles_driven + 30, but doesn't need to >> be named since it's not referenced anywhere else. >> >> BEGIN TRANSACTION; >> UPDATE users SET miles_driven += 30 WHERE name='blake'; >> UPDATE cars SET miles_driven += 30 WHERE model='pinto' AS car; >> COMMIT TRANSACTION IF car.is_running; >> >> Here’s another example, performing the canonical bank transfer: >> >> BEGIN TRANSACTION; >> UPDATE accounts SET balance += 100 WHERE name='blake' AS blake; >> UPDATE accounts SET balance -= 100 WHERE name='benedict' AS benedict; >> COMMIT TRANSACTION IF blake EXISTS AND benedict.balance >= 100; >> >> As you can see from the examples, column values can be referenced via a dot >> syntax, ie: . -> select1.value. Since the read portion >> of the transaction is performed before evaluating conditions or applying >> updates, values read can be freely applied to non-primary key values in >> updates. Select statements used either in checking a condition or creating >> an update must be restricted to a single row, either by specifying the full >> primary key or a limit of 1. Multi-row selects are allowed, but only for >> returning data to the client (see below). >> >> For evaluating conditions, = & != are available for all types, <, <=, >, >= >> are available for numerical types, and EXISTS, NOT EXISTS can be used for >> partitions, rows, and values. If any column references cannot be satisfied >> by the result of the reads, the condition implicitly fails. This prevents >> having to include a bunch of exists statements. > > Is there a new keyword for “partition (not) exists” or is it inferred by the > select? > >> >> On completion, an operation would return a boolean value indicating the >> operation had been applied, and a result set for each named select (but not >> named update). We could also support an optional RETURN keyword, which would >> allow the user to only return specific named selects (ie: RETURN select1, >> select2). >> > > The returned result set is after the updates are applied? > > >> Let me know what you think! >> >> Blake
Re: CEP-15 multi key transaction syntax
> On Jun 3, 2022, at 8:39 AM, Blake Eggleston wrote: > > Hi dev@, First, I’m ridiculously excited to see this. > > I’ve been working on a draft syntax for Accord transactions and wanted to > bring what I have to the dev list to solicit feedback and build consensus > before moving forward with it. The proposed transaction syntax is intended to > be an extended batch syntax. Basically batches with selects, and an optional > condition at the end. To facilitate conditions against an arbitrary number of > select statements, you can also name the statements, and reference columns in > the results. To cut down on the number of operations needed, select values > can also be used in updates, including some math operations. Parameterization > of literals is supported the same as other statements. > > Here's an example selecting a row from 2 tables, and issuing updates for each > row if a condition is met: > > BEGIN TRANSACTION; > SELECT * FROM users WHERE name='blake' AS user; > SELECT * from cars WHERE model='pinto' AS car; > UPDATE users SET miles_driven = user.miles_driven + 30 WHERE name='blake'; > UPDATE cars SET miles_driven = car.miles_driven + 30 WHERE model='pinto'; > COMMIT TRANSACTION IF car.is_running; > > This can be simplified by naming the updates with an AS syntax. If > updates are named, a corresponding read is generated behind the scenes and > its values inform the update. > > Here's an example, the query is functionally identical to the previous query. > In the case of the user update, a read is still performed behind the scenes > to enable the calculation of miles_driven + 30, but doesn't need to be named > since it's not referenced anywhere else. > > BEGIN TRANSACTION; > UPDATE users SET miles_driven += 30 WHERE name='blake'; > UPDATE cars SET miles_driven += 30 WHERE model='pinto' AS car; > COMMIT TRANSACTION IF car.is_running; > > Here’s another example, performing the canonical bank transfer: > > BEGIN TRANSACTION; > UPDATE accounts SET balance += 100 WHERE name='blake' AS blake; > UPDATE accounts SET balance -= 100 WHERE name='benedict' AS benedict; > COMMIT TRANSACTION IF blake EXISTS AND benedict.balance >= 100; > > As you can see from the examples, column values can be referenced via a dot > syntax, ie: . -> select1.value. Since the read portion > of the transaction is performed before evaluating conditions or applying > updates, values read can be freely applied to non-primary key values in > updates. Select statements used either in checking a condition or creating an > update must be restricted to a single row, either by specifying the full > primary key or a limit of 1. Multi-row selects are allowed, but only for > returning data to the client (see below). > > For evaluating conditions, = & != are available for all types, <, <=, >, >= > are available for numerical types, and EXISTS, NOT EXISTS can be used for > partitions, rows, and values. If any column references cannot be satisfied by > the result of the reads, the condition implicitly fails. This prevents having > to include a bunch of exists statements. Is there a new keyword for “partition (not) exists” or is it inferred by the select? > > On completion, an operation would return a boolean value indicating the > operation had been applied, and a result set for each named select (but not > named update). We could also support an optional RETURN keyword, which would > allow the user to only return specific named selects (ie: RETURN select1, > select2). > The returned result set is after the updates are applied? > Let me know what you think! > > Blake
Re: [DISCUSS] Change code style guide WRT to @Override in subclasses / interface implementations
sounds good. Lazy consensus it is. > On Jun 4, 2022, at 11:09 AM, bened...@apache.org wrote: > > > I think lazy consensus is good enough here, since there has been no dissent > so far as I can tell. It’s easier to modify if we assume lazy consensus until > a dispute arises. If anyone wants to escalate to a formal vote, feel free to > say so. > > I’ll update the wiki in a couple of days; we can always roll back if a > dissenting voice appears. > > > From: Dinesh Joshi > Date: Friday, 3 June 2022 at 18:34 > To: dev@cassandra.apache.org > Subject: Re: [DISCUSS] Change code style guide WRT to @Override in subclasses > / interface implementations > > Let’s bring it to vote? We can update the docs as we evolve the guidance but > I think it’s in a good enough shape to publish. > > > On Jun 3, 2022, at 9:07 AM, bened...@apache.org wrote: > > > I always ask if we’re ready, get a few acks, then one or two new queries come > out of the woodwork. > > Perhaps I will just publish, and we can start addressing these queries in a > follow-up process. > > From: Dinesh Joshi > Date: Friday, 3 June 2022 at 16:57 > To: dev@cassandra.apache.org > Subject: Re: [DISCUSS] Change code style guide WRT to @Override in subclasses > / interface implementations > > I don’t think the guide has yet been published to the official website, has > it? Maybe we should just get it out there. > > On Jun 3, 2022, at 8:54 AM, bened...@apache.org wrote: > > > Somebody hasn’t looked at the new style guide*, the conversation for which > keeps rolling on and so it never quite gets promoted to the wiki. It says: > > Always use @Override annotations when implementing abstract or interface > methods or overriding a parent method. > > * > https://docs.google.com/document/d/1sjw0crb0clQin2tMgZLt_ob4hYfLJYaU4lRX722htTo > > > From: Josh McKenzie > Date: Friday, 3 June 2022 at 16:14 > To: dev@cassandra.apache.org > Subject: Re: [DISCUSS] Change code style guide WRT to @Override in subclasses > / interface implementations > > > Avoid redundant @Override annotations when implementing abstract or > > interface methods. > I'd argue they're not redundant. We're humans and infinitely fallible. :) > > +1 to changing this to just always annotate for all the reasons you enumerate. > > On Fri, Jun 3, 2022, at 10:16 AM, Alex Petrov wrote: > Right, my thinking matches what David has mentioned: > > https://issues.apache.org/jira/browse/CASSANDRA-16096 > https://lists.apache.org/thread/mkskwxn921t5bkfmnog032qvnyjk82t7 > > I'll make sure to update the style guide itself, too, since it looks like > there was a vote, and intellij file is updated, just need to fixup the > website. > > > On Fri, Jun 3, 2022, at 4:02 PM, Dinesh Joshi wrote: > So your proposal is to always add override annotation? Or are there > situations where you don’t want to add them? > > > On Jun 3, 2022, at 6:53 AM, Alex Petrov wrote: > > Hi everyone, > > In our style guide [1], we have a following statement: > > > Avoid redundant @Override annotations when implementing abstract or > > interface methods. > > I'd like to suggest we change this. > > @Override annotation in subclasses might be annoying when you're writing the > code for the first time, or reading already familiar code, but when you're > working on large changes and have complex class hierarchies, or multiple > overloads for the method, it's easy to overlook methods that were not marked > as overrides, and leave a wrong method in the code, or misinterpret the call > chain. > > I think @Override annotations are extremely useful and serve their purpose, > especially when refactoring: I can change the interface, and will not only be > pointed to all classes that do not implement the new version (which compiler > will do anyways), but also will be pointed to the classes that, to the human > eye, may look like they're overriding the method, but in fact they do not. > > More concrete example: there is an abstract class between the interface and a > concrete implementation: you change the interface, modify the method in the > abstract class, but then forget to change the signature in the overriden > implementation of the concrete class, and get a behaviour from the abstract > class rather then concrete implementation. > > The question is not about taste or code aesthetics, but about making > maintaining a large codebase that has a lot of complexity and that was > evolving over many years simpler. If you could provide an example where > @Override would be counter-productive or overly burdensome, we could compare > this cost of maintenance with the cost of potential errors. > > Thank you, > --Alex > > [1] https://cassandra.apache.org/_/development/code_style.html > >
Re: [DISCUSS] Change code style guide WRT to @Override in subclasses / interface implementations
I think lazy consensus is good enough here, since there has been no dissent so far as I can tell. It’s easier to modify if we assume lazy consensus until a dispute arises. If anyone wants to escalate to a formal vote, feel free to say so. I’ll update the wiki in a couple of days; we can always roll back if a dissenting voice appears. From: Dinesh Joshi Date: Friday, 3 June 2022 at 18:34 To: dev@cassandra.apache.org Subject: Re: [DISCUSS] Change code style guide WRT to @Override in subclasses / interface implementations Let’s bring it to vote? We can update the docs as we evolve the guidance but I think it’s in a good enough shape to publish. On Jun 3, 2022, at 9:07 AM, bened...@apache.org wrote: I always ask if we’re ready, get a few acks, then one or two new queries come out of the woodwork. Perhaps I will just publish, and we can start addressing these queries in a follow-up process. From: Dinesh Joshi Date: Friday, 3 June 2022 at 16:57 To: dev@cassandra.apache.org Subject: Re: [DISCUSS] Change code style guide WRT to @Override in subclasses / interface implementations I don’t think the guide has yet been published to the official website, has it? Maybe we should just get it out there. On Jun 3, 2022, at 8:54 AM, bened...@apache.org wrote: Somebody hasn’t looked at the new style guide*, the conversation for which keeps rolling on and so it never quite gets promoted to the wiki. It says: Always use @Override annotations when implementing abstract or interface methods or overriding a parent method. * https://docs.google.com/document/d/1sjw0crb0clQin2tMgZLt_ob4hYfLJYaU4lRX722htTo From: Josh McKenzie Date: Friday, 3 June 2022 at 16:14 To: dev@cassandra.apache.org Subject: Re: [DISCUSS] Change code style guide WRT to @Override in subclasses / interface implementations > Avoid redundant @Override annotations when implementing abstract or interface > methods. I'd argue they're not redundant. We're humans and infinitely fallible. :) +1 to changing this to just always annotate for all the reasons you enumerate. On Fri, Jun 3, 2022, at 10:16 AM, Alex Petrov wrote: Right, my thinking matches what David has mentioned: https://issues.apache.org/jira/browse/CASSANDRA-16096 https://lists.apache.org/thread/mkskwxn921t5bkfmnog032qvnyjk82t7 I'll make sure to update the style guide itself, too, since it looks like there was a vote, and intellij file is updated, just need to fixup the website. On Fri, Jun 3, 2022, at 4:02 PM, Dinesh Joshi wrote: So your proposal is to always add override annotation? Or are there situations where you don’t want to add them? On Jun 3, 2022, at 6:53 AM, Alex Petrov wrote: Hi everyone, In our style guide [1], we have a following statement: > Avoid redundant @Override annotations when implementing abstract or interface > methods. I'd like to suggest we change this. @Override annotation in subclasses might be annoying when you're writing the code for the first time, or reading already familiar code, but when you're working on large changes and have complex class hierarchies, or multiple overloads for the method, it's easy to overlook methods that were not marked as overrides, and leave a wrong method in the code, or misinterpret the call chain. I think @Override annotations are extremely useful and serve their purpose, especially when refactoring: I can change the interface, and will not only be pointed to all classes that do not implement the new version (which compiler will do anyways), but also will be pointed to the classes that, to the human eye, may look like they're overriding the method, but in fact they do not. More concrete example: there is an abstract class between the interface and a concrete implementation: you change the interface, modify the method in the abstract class, but then forget to change the signature in the overriden implementation of the concrete class, and get a behaviour from the abstract class rather then concrete implementation. The question is not about taste or code aesthetics, but about making maintaining a large codebase that has a lot of complexity and that was evolving over many years simpler. If you could provide an example where @Override would be counter-productive or overly burdensome, we could compare this cost of maintenance with the cost of potential errors. Thank you, --Alex [1] https://cassandra.apache.org/_/development/code_style.html
