SqlCompactErrorLog assumes that if the database file exists, it has the elmah tables

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Use elmah on a ASP.NET project that already uses SQL Server CE 4.0
2. Go to the elmah page
3. It fails.

What is the expected output? What do you see instead?
Expected: elmah page.
Instead: error.

What version of the product are you using? On what operating system?
elmah 1.2 beta

Please provide any additional information below.
The SqlCompactErrorLog.InitializeDatabase() method assumes (twice) that if the 
database file exists, it must already have the elmah tables. This is not always 
the case. So, the simplest alternative could be to verify that the table really 
exists (once you verify that the database file exists), instead of assuming 
that it does.

Original issue reported on code.google.com by je...@garzazambrano.net on 28 Apr 2011 at 8:15

[GoogleCodeExporter]

Original comment by azizatif on 28 Apr 2011 at 10:51

• Added labels: Component-Persistence, Milestone-Release1.2

[GoogleCodeExporter]

This is already fixed here, as far as I can see: 
http://code.google.com/p/elmah/source/browse/branches/RB-1.2/src/Elmah/SqlServer
CompactErrorLog.cs?spec=svn817&r=817

Original comment by ejls...@hotmail.com on 29 Apr 2011 at 8:26

[GoogleCodeExporter]

Yes, you are right. I guess I took another version. Thank you.

Best regards.

Original comment by je...@garzazambrano.net on 29 Apr 2011 at 2:39

[GoogleCodeExporter]

Original comment by azizatif on 29 Apr 2011 at 7:31

• Changed state: Duplicate

[GoogleCodeExporter]

I tested against the nuget release and I see the same error. I am looking this 
on the source:

private void InitializeDatabase()
        {
            string connectionString = ConnectionString;
            Debug.AssertStringNotEmpty(connectionString);

            string dbFilePath = ConnectionStringHelper.GetDataSourceFilePath(connectionString);
            if (File.Exists(dbFilePath))
                return;

The issue I reported is still there, isn't it? It assumes that if the database 
file exists, it must already have the elmah tables. 

Original comment by je...@garzazambrano.net on 3 Jun 2011 at 4:46

[GoogleCodeExporter]

Yes, that is true. All other file based Log implementations do the same. 
(Return if the file exists)

Original comment by ejls...@hotmail.com on 3 Jun 2011 at 5:35

[GoogleCodeExporter]

So, instead of validating the existence of the database, the method should 
validate the existence of the table:

SELECT * FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = 'ELMAH_Error'
or
SELECT COUNT(*) FROM ...

Original comment by je...@garzazambrano.net on 3 Jun 2011 at 6:37

[GoogleCodeExporter]

I am trying the first one with SqlCe40Toolbox and its crashing the program. The 
second one works:

SELECT COUNT(*) FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = 'ELMAH_Error'

Original comment by je...@garzazambrano.net on 3 Jun 2011 at 6:41

[GoogleCodeExporter]

Please address support issues for my SQL Server Compact Toolbox here: 
http://sqlcetoolbox/codeplex.com

Original comment by ejls...@hotmail.com on 4 Jun 2011 at 9:45

[GoogleCodeExporter]

Please don't mind my comment about SqlCe40Toolbox, it was just to say that the 
second command is tested against sql server ce and it works to verify that the 
ELMAH_Error table exists.

Original comment by je...@garzazambrano.net on 4 Jun 2011 at 5:05

[GoogleCodeExporter]

So does the first one...

Original comment by ejls...@hotmail.com on 5 Jun 2011 at 8:05

[GoogleCodeExporter]

Fixed in r900 in the SP1 branch

Original comment by ejls...@hotmail.com on 8 Jun 2011 at 3:00

[GoogleCodeExporter]

This appears not to be a duplicate of an issue #216.

Original comment by azizatif on 9 Jun 2011 at 5:46

• Changed state: New

[GoogleCodeExporter]

Responding to parallel thread on discussion group[1]:

> The overhead if opening the database connection 
> each time the constructor is called

This overhead can be significant and should be a basis for decision. An even 
bigger problem to avoid is race conditions. What if two threads run the 
constructor at the same time and find the table is not there and then go out 
and try to create it at the same time! One of the two will result in an 
undesired exception! It would be a good idea, therefore, to guarantee an atomic 
initialization of the database. You should, for example, handle the distinct 
case that creating the table will fail with an error stating that the object 
already exists! Does SQL Server Compact have a distinct error type/code for 
this?

> not sure if that is a real issue
> how often is the constructor called

Assume the worst case, which is very often. There is no control over this.

> I wouldn't worry too much about the constructor... 
> in the normal course of play, ELMAH will only call this on 
> application start up.

ELMAH can and will call the constructor as needed and the application hosting 
ELMAH should be free to do so too. ELMAH certainly does not do much at start-up 
time. The ErrorLogModule may be initialized at start-up by ASP.NET but the 
ErrorLogModule never uses, caches or calls into the configured error log 
implementation until an error occurs. When that happens, an ErrorLog subclass 
is constructed!

> If I implement the suggested change, all other file based 
> providers should be changed as well, or?

Ideally but not required. I'd like to think that each error log implementation 
has some liberty in terms of surfacing the benefits of the underlying storage 
and its implementation choices.

> Even the SQL Server based provider assumes the tables 
> to be always present…

It is a different beast for security reasons but keeping the constructor 
lightweight was a major reason as well. Yet another reason was to allow people 
to tailor the script according to their special needs as long as they respect 
the input and output of the stored procedures.

The only reason you would have a file but no table is if someone is trying to 
reuse an existing DB file for error logging as well. This should be a rare 
scenario with database implementations like SQL Server Compact where having 
separate database files is not a major issue (rather an advantage). In light of 
this, I'm changing the type of issue to be an enhancement rather than an 
inherent defect. An ideal implementation of this issue would require some major 
re-work and testing and is therefore risky to roll out in a service pack 
release (removing milestone 1.2). An ideal implementation would take advantage 
of the fact that DB/table creation is only significant when logging an error. 
Logging can be seen as something expensive so the DB initialization and table 
existence check could be moved into into the Log method. The GetError and 
GetErrors methods could be rewired to return nothing when the file or table 
does not exist. It is cheap to check for file existence but table existence 
during the reading function could be handled as a distinct exception when 
running the SELECT query.

[1] http://groups.google.com/group/elmah/t/a8bba16290312a7a?hl=en

Original comment by azizatif on 9 Jun 2011 at 6:32

• Added labels: Type-Enhancement
• Removed labels: Milestone-Release1.2, Type-Defect

[GoogleCodeExporter]

I suggest undoing the fix then - how do I do that, make a new update?

Original comment by ejls...@hotmail.com on 9 Jun 2011 at 8:35

[GoogleCodeExporter]

You can undo a revision by doing a reserve merge. If you are using TortoiseSVN, 
you can do the same by viewing the log via "Show log", selecting a revision 
(r900 in this case, I think) and then choose "Revert changes from this 
revision" from the context-menu.

Original comment by azizatif on 9 Jun 2011 at 3:29

[GoogleCodeExporter]

Undone in r901

Original comment by ejls...@hotmail.com on 9 Jun 2011 at 4:13