Jan Pazdziora wrote:
Another email, focusing on the tables/ directory.
On Tue, May 12, 2009 at 12:42:32PM -0400, Jeff Ortel wrote:
- Table files (rhnsat/tables/*)
- <table>.sql files:
- Most moved to common/tables/
- Trigger DDL split out and moved into existing or new
oracle/triggers/<table>.sql
- *_triggers.sql files:
- (git) moved to oracle/triggers/
- _triggers suffix removed because redundant w/ directory scoping.
- *_data.sql files:
- Most (git) moved to common/data/
- Few files forked and (git) moved to oracle/data. Forked files copied to
postgres/data and ported to PG.
- _data suffix removed because redundant w/ directory scoping.
- *_index.sql files consolidated into file w/ table DDL. Most of the indexes
were already there anyway.
This all sound like a sane to do for just Oracle anyway. Polishing the
schema sources is always good thing, so I propose doing the changes
in master where we can, one issue per commit, without waiting for
PostgreSQL-specific parts to be ready.
I thought so too :)
I see the following problems:
The rhn_ksscript_mod_trig trigger is still defined on wrong table, see
https://www.redhat.com/archives/spacewalk-devel/2008-September/msg00136.html
That would signal that no validation of the semantics of the code
was done.
The focus during the refactoring was to ensure that the end schema was exactly the same as
the original. This kind of proves that.
I'm very affraid that by doing large shuffles without having
some more formal validation in place, we will lose information and
potentially introduce many bugs.
I'm planning on doing a side-by-side comparison of (2) oracle databases; one installed
from master and the other from pgsql. We should be able to verify that no information was
lost.
I also wonder if this would be a good
time to generate the
for each row
begin
:new.modified := sysdate;
end;
and other common types of triggers automatically.
Might be better as a follow on.
There seem to be not only moves of table DDL tables but also changes
in the content. Things like
-create table
-web_user_site_type
+
+CREATE TABLE web_user_site_type
(
- type char(1)
- constraint wust_type_nn not null
- constraint wust_type_pk primary key,
- description varchar2(64)
- constraint wust_desc_nn not null
+ type CHAR(1) NOT NULL
+ CONSTRAINT wust_type_pk PRIMARY KEY,
+ description VARCHAR2(64) NOT NULL
)
- enable row movement
- ;
+ENABLE ROW MOVEMENT
+;
Why were these changes done? They do not seem to be necessary, it's
hard to see what is changing, and again, we potentially introduce
bugs.
Yes, as I mentioned in the summary, the common table files were reformatted. I agree this
was not necessary. But, I decided to use chameleon to help automate refactoring the files
and it renders formatted output. Also, the style and formatting was a hodge-podge and
kind of needed it.
The new code seems to have trailing spaces. Is that some kind of
format=flowed thing or is this a bug which should be addressed?
Seems to load into oracle and postgres just fine. Can you be more specific about why this
is a problem?
This particular patch also shows another issue -- the wust_type_nn
constraint is dropped and pure NOT NULL clause is used. Which means
that the cosntraint will have generated (SYS_*) name, which in turn
will cause freshly installed and upgraded schemas to differ, making
the validation hard(er) and more error-prone. Please do not remove
those names. Isn't chameleon's optimizer supposed to do just that in
build time?
Yes, but having named NOT NULL constraints is terribly unnecessary and only clutters up
the schema. As far as upgrades, I considered this. In oracle, a table created with a
named not null constraint can be altered as (alter table person modify age null) to remove
the not null constraint on the 'age' column. Keeping this in mind, this should not be a
problem.
I can see that you've renamed tables/Makefile.deps to
tables/tables.deps while also changing the indentation and introducing
some extra trailing spaces.
Since these files are now used by 'blend' and not Make, I renamed to avoid confusion as to
what they are for. Also, the formatting in these files was terrible and after working
with them for weeks, I just had to fix them. I was very careful to keep them correct and
the schema loads just find.
Again, it makes it very hard to see what
has happened.
I understand.
I propose reverting the rename and/or doing the rename
in master in one commit, and then the indentation in another commit,
so that diff among the branches is possible.
Shouldn't we be able to generate the dependency tree of tables
automatically and render the tables/tables.deps file obsolete? With
triggers removed from tables' DDL files, parsing the foreign key
definitions shouldn't be that hard.
For the tables, yes but not for view, procs, packages, types and classes.
Why is content file rhnArchTypeActions.sql duplicated both in
oracle/data and in postgres/rhnArchTypeActions.sqldata when the file itself is
the same? Why
is it not in common/data?
It contains sub-queries which chameleon cannot handle. At this point, I wanted to limit
the scope of chameleon's grammar to DDL and simple SQL. We can consider expanding this
later and move the file to common.
The file rhnBlacklistObsoletes.sql has the
same problem.
Same explanation as ^^ rhnArchTypeActions.sql ^^.
The rest of the files in oracle/data seems to only
differ from the postgres content in the sequence's nextval syntax.
Chameleon does handle and convert sequence nextval syntax.
Yet, many files in common/data have the Oracle syntax just fine, and
looking at chameleon's code, this should again be a build time issue.
I assume you want to review the database-specific data directories and
put most of the cotnent back to common/data.
_______________________________________________
Spacewalk-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/spacewalk-devel
